diff --git a/checkpoint.json b/checkpoint.json new file mode 100644 index 000000000..6276acfca --- /dev/null +++ b/checkpoint.json @@ -0,0 +1,57 @@ +{ + "timestamp": "2026-03-16T12:01:53.791317Z", + "current_node": "setup", + "completed_nodes": [ + "start", + "setup" + ], + "node_retries": { + "setup": 1, + "start": 1 + }, + "context_values": { + "failure_class": "deterministic", + "current_node": "setup", + "failure_signature": "setup|deterministic|script failed with exit code: ## stdout fatal: destination path '.' already exists and is not an empty directory.", + "internal.retry_count.setup": 1, + "command.output": "fatal: destination path '.' already exists and is not an empty directory.\n", + "internal.thread_id": "start", + "outcome": "fail", + "internal.retry_count.start": 1, + "internal.fidelity": "compact", + "internal.run_id": "01KKV8C59GRK1DAXY9JE616NZ2", + "command.stderr": "", + "graph.goal": "Prevent using __isnull lookup with non-boolean value.\nDescription\n\t \n\t\t(last modified by Mariusz Felisiak)\n\t \n__isnull should not allow for non-boolean values. Using truthy/falsey doesn't promote INNER JOIN to an OUTER JOIN but works fine for a simple queries. Using non-boolean values is ​undocumented and untested. IMO we should raise an error for non-boolean values to avoid confusion and for consistency.\n\n\n\n## Additional Context\n\nPR here: ​https://github.com/django/django/pull/11873\nAfter the reconsideration I don't think that we should change this ​documented behavior (that is in Django from the very beginning). __isnull lookup expects boolean values in many places and IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples field__isnull='false' or field__isnull='true' (both would return the same result). You can always call bool() on a right hand side. Sorry for my previous acceptation (I shouldn't triage tickets in the weekend).\nReplying to felixxm: After the reconsideration I don't think that we should change this ​documented behavior (that is in Django from the very beginning). __isnull lookup expects boolean values in many places and IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples field__isnull='false' or field__isnull='true' (both would return the same result). You can always call bool() on a right hand side. Sorry for my previous acceptation (I shouldn't triage tickets in the weekend). I understand your point. But is there anything we can do to avoid people falling for the same pitfall I did? The problem, in my opinion, is that it works fine for simple queries but as soon as you add a join that needs promotion it will break, silently. Maybe we should make it raise an exception when a non-boolean is passed? One valid example is to have a class that implements __bool__. You can see here ​https://github.com/django/django/blob/d9881a025c15d87b2a7883ee50771117450ea90d/django/db/models/lookups.py#L465-L470 that non-bool value is converted to IS NULL and IS NOT NULL already using the truthy/falsy values. IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples fieldisnull='false' or fieldisnull='true' (both would return the same result). This is already the case. It just is inconsistent, in lookups.py field__isnull='false' will be a positive condition but on the query.py it will be the negative condition.\nMaybe adding a note on the documentation? something like: \"Although it might seem like it will work with non-bool fields, this is not supported and can lead to inconsistent behaviours\"\nAgreed, we should raise an error for non-boolean values, e.g. diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 9344979c56..fc4a38c4fe 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -463,6 +463,11 @@ class IsNull(BuiltinLookup): prepare_rhs = False def as_sql(self, compiler, connection): + if not isinstance(self.rhs, bool): + raise ValueError( + 'The QuerySet value for an isnull lookup must be True or ' + 'False.' + ) sql, params = compiler.compile(self.lhs) if self.rhs: return \"%s IS NULL\" % sql, params I changed the ticket description.\nThanks, I'll work on it! Wouldn't that possibly break backward compatibility? I'm not familiar with how Django moves in that regard.\nWe can add a release note in \"Backwards incompatible changes\" or deprecate this and remove in Django 4.0. I have to thing about it, please give me a day, maybe I will change my mind :)\nNo problem. Thanks for taking the time to look into this!\nAnother interesting example related to this: As an anecdote, I've also got bitten by this possibility. An attempt to write WHERE (field IS NULL) = boolean_field as .filter(field__isnull=F('boolean_field')) didn't go as I expected. Alexandr Aktsipetrov -- ​https://groups.google.com/forum/#!msg/django-developers/AhY2b3rxkfA/0sz3hNanCgAJ This example will generate the WHERE .... IS NULL. I guess we also would want an exception thrown here.\nAndré, IMO we should deprecate using non-boolean values in Django 3.1 (RemovedInDjango40Warning) and remove in Django 4.0 (even if it is untested and undocumented). I can imagine that a lot of people use e.g. 1 and 0 instead of booleans. Attached diff fixes also issue with passing a F() expression. def as_sql(self, compiler, connection): if not isinstance(self.rhs, bool): raise RemovedInDjango40Warning(...) ....\nReplying to felixxm: André, IMO we should deprecate using non-boolean values in Django 3.1 (RemovedInDjango40Warning) and remove in Django 4.0 (even if it is untested and undocumented). I can imagine that a lot of people use e.g. 1 and 0 instead of booleans. Attached diff fixes also issue with passing a F() expression. def as_sql(self, compiler, connection): if not isinstance(self.rhs, bool): raise RemovedInDjango40Warning(...) .... Sound like a good plan. Not super familiar with the branch structure of Django. So, I guess the path to follow is to make a PR to master adding the deprecation warning and eventually when master is 4.x we create the PR raising the ValueError. Is that right? Thanks!\nAndré, yes mostly. You can find more details about that ​from the documentation.", + "current.preamble": "Goal: Prevent using __isnull lookup with non-boolean value.\nDescription\n\t \n\t\t(last modified by Mariusz Felisiak)\n\t \n__isnull should not allow for non-boolean values. Using truthy/falsey doesn't promote INNER JOIN to an OUTER JOIN but works fine for a simple queries. Using non-boolean values is ​undocumented and untested. IMO we should raise an error for non-boolean values to avoid confusion and for consistency.\n\n\n\n## Additional Context\n\nPR here: ​https://github.com/django/django/pull/11873\nAfter the reconsideration I don't think that we should change this ​documented behavior (that is in Django from the very beginning). __isnull lookup expects boolean values in many places and IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples field__isnull='false' or field__isnull='true' (both would return the same result). You can always call bool() on a right hand side. Sorry for my previous acceptation (I shouldn't triage tickets in the weekend).\nReplying to felixxm: After the reconsideration I don't think that we should change this ​documented behavior (that is in Django from the very beginning). __isnull lookup expects boolean values in many places and IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples field__isnull='false' or field__isnull='true' (both would return the same result). You can always call bool() on a right hand side. Sorry for my previous acceptation (I shouldn't triage tickets in the weekend). I understand your point. But is there anything we can do to avoid people falling for the same pitfall I did? The problem, in my opinion, is that it works fine for simple queries but as soon as you add a join that needs promotion it will break, silently. Maybe we should make it raise an exception when a non-boolean is passed? One valid example is to have a class that implements __bool__. You can see here ​https://github.com/django/django/blob/d9881a025c15d87b2a7883ee50771117450ea90d/django/db/models/lookups.py#L465-L470 that non-bool value is converted to IS NULL and IS NOT NULL already using the truthy/falsy values. IMO it would be confusing if we'll allow for truthy/falsy values, e.g. take a look at these examples fieldisnull='false' or fieldisnull='true' (both would return the same result). This is already the case. It just is inconsistent, in lookups.py field__isnull='false' will be a positive condition but on the query.py it will be the negative condition.\nMaybe adding a note on the documentation? something like: \"Although it might seem like it will work with non-bool fields, this is not supported and can lead to inconsistent behaviours\"\nAgreed, we should raise an error for non-boolean values, e.g. diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 9344979c56..fc4a38c4fe 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -463,6 +463,11 @@ class IsNull(BuiltinLookup): prepare_rhs = False def as_sql(self, compiler, connection): + if not isinstance(self.rhs, bool): + raise ValueError( + 'The QuerySet value for an isnull lookup must be True or ' + 'False.' + ) sql, params = compiler.compile(self.lhs) if self.rhs: return \"%s IS NULL\" % sql, params I changed the ticket description.\nThanks, I'll work on it! Wouldn't that possibly break backward compatibility? I'm not familiar with how Django moves in that regard.\nWe can add a release note in \"Backwards incompatible changes\" or deprecate this and remove in Django 4.0. I have to thing about it, please give me a day, maybe I will change my mind :)\nNo problem. Thanks for taking the time to look into this!\nAnother interesting example related to this: As an anecdote, I've also got bitten by this possibility. An attempt to write WHERE (field IS NULL) = boolean_field as .filter(field__isnull=F('boolean_field')) didn't go as I expected. Alexandr Aktsipetrov -- ​https://groups.google.com/forum/#!msg/django-developers/AhY2b3rxkfA/0sz3hNanCgAJ This example will generate the WHERE .... IS NULL. I guess we also would want an exception thrown here.\nAndré, IMO we should deprecate using non-boolean values in Django 3.1 (RemovedInDjango40Warning) and remove in Django 4.0 (even if it is untested and undocumented). I can imagine that a lot of people use e.g. 1 and 0 instead of booleans. Attached diff fixes also issue with passing a F() expression. def as_sql(self, compiler, connection): if not isinstance(self.rhs, bool): raise RemovedInDjango40Warning(...) ....\nReplying to felixxm: André, IMO we should deprecate using non-boolean values in Django 3.1 (RemovedInDjango40Warning) and remove in Django 4.0 (even if it is untested and undocumented). I can imagine that a lot of people use e.g. 1 and 0 instead of booleans. Attached diff fixes also issue with passing a F() expression. def as_sql(self, compiler, connection): if not isinstance(self.rhs, bool): raise RemovedInDjango40Warning(...) .... Sound like a good plan. Not super familiar with the branch structure of Django. So, I guess the path to follow is to make a PR to master adding the deprecation warning and eventually when master is 4.x we create the PR raising the ValueError. Is that right? Thanks!\nAndré, yes mostly. You can find more details about that ​from the documentation.\n", + "internal.node_visit_count": 1, + "thread.start.current_node": "setup", + "graph.rankdir": "LR" + }, + "logs": [], + "node_outcomes": { + "start": { + "status": "success", + "duration_ms": 0 + }, + "setup": { + "status": "fail", + "context_updates": { + "command.output": "fatal: destination path '.' already exists and is not an empty directory.\n", + "command.stderr": "" + }, + "failure": { + "message": "Script failed with exit code: 128\n\n## stdout\nfatal: destination path '.' already exists and is not an empty directory.\n", + "failure_class": "deterministic" + }, + "duration_ms": 23 + } + }, + "next_node_id": "solve", + "loop_failure_signatures": { + "setup|deterministic|script failed with exit code: ## stdout fatal: destination path '.' already exists and is not an empty directory.": 1 + }, + "node_visits": { + "start": 1, + "setup": 1 + } +} \ No newline at end of file diff --git a/nodes/setup/script_invocation.json b/nodes/setup/script_invocation.json new file mode 100644 index 000000000..d0d62e070 --- /dev/null +++ b/nodes/setup/script_invocation.json @@ -0,0 +1,5 @@ +{ + "command": "git clone https://github.com/django/django.git . && git checkout 2f72480fbd27896c986c45193e1603e35c0b19a7 && python -m pip install -e .", + "language": "shell", + "timeout_ms": null +} \ No newline at end of file diff --git a/nodes/setup/script_timing.json b/nodes/setup/script_timing.json new file mode 100644 index 000000000..1854d02dc --- /dev/null +++ b/nodes/setup/script_timing.json @@ -0,0 +1,5 @@ +{ + "duration_ms": 23, + "exit_code": 128, + "timed_out": false +} \ No newline at end of file diff --git a/nodes/setup/status.json b/nodes/setup/status.json new file mode 100644 index 000000000..5709608e2 --- /dev/null +++ b/nodes/setup/status.json @@ -0,0 +1,6 @@ +{ + "status": "fail", + "notes": null, + "failure_reason": "Script failed with exit code: 128\n\n## stdout\nfatal: destination path '.' already exists and is not an empty directory.\n", + "timestamp": "2026-03-16T12:01:53.791166+00:00" +} \ No newline at end of file diff --git a/nodes/start/status.json b/nodes/start/status.json new file mode 100644 index 000000000..3f4dcedad --- /dev/null +++ b/nodes/start/status.json @@ -0,0 +1,6 @@ +{ + "status": "success", + "notes": null, + "failure_reason": null, + "timestamp": "2026-03-16T12:01:53.766666+00:00" +} \ No newline at end of file