mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
parent
22b9c98b0b
commit
651be2ff84
5 changed files with 79 additions and 0 deletions
57
checkpoint.json
Normal file
57
checkpoint.json
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
{
|
||||
"timestamp": "2026-03-16T12:08:36.110975Z",
|
||||
"current_node": "setup",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"setup"
|
||||
],
|
||||
"node_retries": {
|
||||
"setup": 1,
|
||||
"start": 1
|
||||
},
|
||||
"context_values": {
|
||||
"thread.start.current_node": "setup",
|
||||
"current.preamble": "Goal: Filtering on query result overrides GROUP BY of internal query\nDescription\n\t\nfrom django.contrib.auth import models\na = models.User.objects.filter(email__isnull=True).values('email').annotate(m=Max('id')).values('m')\nprint(a.query) # good\n# SELECT MAX(\"auth_user\".\"id\") AS \"m\" FROM \"auth_user\" WHERE \"auth_user\".\"email\" IS NULL GROUP BY \"auth_user\".\"email\"\nprint(a[:1].query) # good\n# SELECT MAX(\"auth_user\".\"id\") AS \"m\" FROM \"auth_user\" WHERE \"auth_user\".\"email\" IS NULL GROUP BY \"auth_user\".\"email\" LIMIT 1\nb = models.User.objects.filter(id=a[:1])\nprint(b.query) # GROUP BY U0.\"id\" should be GROUP BY U0.\"email\"\n# SELECT ... FROM \"auth_user\" WHERE \"auth_user\".\"id\" = (SELECT U0.\"id\" FROM \"auth_user\" U0 WHERE U0.\"email\" IS NULL GROUP BY U0.\"id\" LIMIT 1)\n\n\n\n## Additional Context\n\nWorkaround: from django.contrib.auth import models a = models.User.objects.filter(email__isnull=True).values('email').aggregate(Max('id'))['id_max'] b = models.User.objects.filter(id=a)\nThanks for tackling that one James! If I can provide you some guidance I'd suggest you have a look at lookups.Exact.process_rhs https://github.com/django/django/blob/ea25bdc2b94466bb1563000bf81628dea4d80612/django/db/models/lookups.py#L265-L267 We probably don't want to perform the clear_select_clause and add_fields(['pk']) when the query is already selecting fields. That's exactly what In.process_rhs does already by only performing these operations if not getattr(self.rhs, 'has_select_fields', True).\nThanks so much for the help Simon! This is a great jumping-off point. There's something that I'm unclear about, which perhaps you can shed some light on. While I was able to replicate the bug with 2.2, when I try to create a test on Master to validate the bug, the group-by behavior seems to have changed. Here's the test that I created: def test_exact_selected_field_rhs_subquery(self): author_1 = Author.objects.create(name='one') author_2 = Author.objects.create(name='two') max_ids = Author.objects.filter(alias__isnull=True).values('alias').annotate(m=Max('id')).values('m') authors = Author.objects.filter(id=max_ids[:1]) self.assertFalse(str(max_ids.query)) # This was just to force the test-runner to output the query. self.assertEqual(authors[0], author_2) And here's the resulting query: SELECT MAX(\"lookup_author\".\"id\") AS \"m\" FROM \"lookup_author\" WHERE \"lookup_author\".\"alias\" IS NULL GROUP BY \"lookup_author\".\"alias\", \"lookup_author\".\"name\" It no longer appears to be grouping by the 'alias' field listed in the initial .values() preceeding the .annotate(). I looked at the docs and release notes to see if there was a behavior change, but didn't see anything listed. Do you know if I'm just misunderstanding what's happening here? Or does this seem like a possible regression?\nIt's possible that a regression was introduced in between. Could you try bisecting the commit that changed the behavior https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#bisecting-a-regression\nMmm actually disregard that. The second value in the GROUP BY is due to the ordering value in the Author class's Meta class. class Author(models.Model): name = models.CharField(max_length=100) alias = models.CharField(max_length=50, null=True, blank=True) class Meta: ordering = ('name',) Regarding the bug in question in this ticket, what should the desired behavior be if the inner query is returning multiple fields? With the fix, which allows the inner query to define a field to return/group by, if there are multiple fields used then it will throw a sqlite3.OperationalError: row value misused. Is this the desired behavior or should it avoid this problem by defaulting back to pk if more than one field is selected?\nI think that we should only default to pk if no fields are selected. The ORM has preliminary support for multi-column lookups and other interface dealing with subqueries doesn't prevent passing queries with multiple fields so I'd stick to the current __in lookup behavior.\n",
|
||||
"graph.goal": "Filtering on query result overrides GROUP BY of internal query\nDescription\n\t\nfrom django.contrib.auth import models\na = models.User.objects.filter(email__isnull=True).values('email').annotate(m=Max('id')).values('m')\nprint(a.query) # good\n# SELECT MAX(\"auth_user\".\"id\") AS \"m\" FROM \"auth_user\" WHERE \"auth_user\".\"email\" IS NULL GROUP BY \"auth_user\".\"email\"\nprint(a[:1].query) # good\n# SELECT MAX(\"auth_user\".\"id\") AS \"m\" FROM \"auth_user\" WHERE \"auth_user\".\"email\" IS NULL GROUP BY \"auth_user\".\"email\" LIMIT 1\nb = models.User.objects.filter(id=a[:1])\nprint(b.query) # GROUP BY U0.\"id\" should be GROUP BY U0.\"email\"\n# SELECT ... FROM \"auth_user\" WHERE \"auth_user\".\"id\" = (SELECT U0.\"id\" FROM \"auth_user\" U0 WHERE U0.\"email\" IS NULL GROUP BY U0.\"id\" LIMIT 1)\n\n\n\n## Additional Context\n\nWorkaround: from django.contrib.auth import models a = models.User.objects.filter(email__isnull=True).values('email').aggregate(Max('id'))['id_max'] b = models.User.objects.filter(id=a)\nThanks for tackling that one James! If I can provide you some guidance I'd suggest you have a look at lookups.Exact.process_rhs https://github.com/django/django/blob/ea25bdc2b94466bb1563000bf81628dea4d80612/django/db/models/lookups.py#L265-L267 We probably don't want to perform the clear_select_clause and add_fields(['pk']) when the query is already selecting fields. That's exactly what In.process_rhs does already by only performing these operations if not getattr(self.rhs, 'has_select_fields', True).\nThanks so much for the help Simon! This is a great jumping-off point. There's something that I'm unclear about, which perhaps you can shed some light on. While I was able to replicate the bug with 2.2, when I try to create a test on Master to validate the bug, the group-by behavior seems to have changed. Here's the test that I created: def test_exact_selected_field_rhs_subquery(self): author_1 = Author.objects.create(name='one') author_2 = Author.objects.create(name='two') max_ids = Author.objects.filter(alias__isnull=True).values('alias').annotate(m=Max('id')).values('m') authors = Author.objects.filter(id=max_ids[:1]) self.assertFalse(str(max_ids.query)) # This was just to force the test-runner to output the query. self.assertEqual(authors[0], author_2) And here's the resulting query: SELECT MAX(\"lookup_author\".\"id\") AS \"m\" FROM \"lookup_author\" WHERE \"lookup_author\".\"alias\" IS NULL GROUP BY \"lookup_author\".\"alias\", \"lookup_author\".\"name\" It no longer appears to be grouping by the 'alias' field listed in the initial .values() preceeding the .annotate(). I looked at the docs and release notes to see if there was a behavior change, but didn't see anything listed. Do you know if I'm just misunderstanding what's happening here? Or does this seem like a possible regression?\nIt's possible that a regression was introduced in between. Could you try bisecting the commit that changed the behavior https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#bisecting-a-regression\nMmm actually disregard that. The second value in the GROUP BY is due to the ordering value in the Author class's Meta class. class Author(models.Model): name = models.CharField(max_length=100) alias = models.CharField(max_length=50, null=True, blank=True) class Meta: ordering = ('name',) Regarding the bug in question in this ticket, what should the desired behavior be if the inner query is returning multiple fields? With the fix, which allows the inner query to define a field to return/group by, if there are multiple fields used then it will throw a sqlite3.OperationalError: row value misused. Is this the desired behavior or should it avoid this problem by defaulting back to pk if more than one field is selected?\nI think that we should only default to pk if no fields are selected. The ORM has preliminary support for multi-column lookups and other interface dealing with subqueries doesn't prevent passing queries with multiple fields so I'd stick to the current __in lookup behavior.",
|
||||
"command.output": "fatal: destination path '.' already exists and is not an empty directory.\n",
|
||||
"internal.thread_id": "start",
|
||||
"internal.node_visit_count": 1,
|
||||
"current_node": "setup",
|
||||
"failure_class": "deterministic",
|
||||
"command.stderr": "",
|
||||
"internal.fidelity": "compact",
|
||||
"graph.rankdir": "LR",
|
||||
"outcome": "fail",
|
||||
"internal.run_id": "01KKV8RCZEQ1VFXCRVP1W2NAX2",
|
||||
"internal.retry_count.setup": 1,
|
||||
"internal.retry_count.start": 1,
|
||||
"failure_signature": "setup|deterministic|script failed with exit code: <n> ## stdout fatal: destination path '.' already exists and is not an empty directory."
|
||||
},
|
||||
"logs": [],
|
||||
"node_outcomes": {
|
||||
"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": 24
|
||||
},
|
||||
"start": {
|
||||
"status": "success",
|
||||
"duration_ms": 0
|
||||
}
|
||||
},
|
||||
"next_node_id": "solve",
|
||||
"loop_failure_signatures": {
|
||||
"setup|deterministic|script failed with exit code: <n> ## stdout fatal: destination path '.' already exists and is not an empty directory.": 1
|
||||
},
|
||||
"node_visits": {
|
||||
"start": 1,
|
||||
"setup": 1
|
||||
}
|
||||
}
|
||||
5
nodes/setup/script_invocation.json
Normal file
5
nodes/setup/script_invocation.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"command": "git clone https://github.com/django/django.git . && git checkout 3346b78a8a872286a245d1e77ef4718fc5e6be1a && python -m pip install -e .",
|
||||
"language": "shell",
|
||||
"timeout_ms": null
|
||||
}
|
||||
5
nodes/setup/script_timing.json
Normal file
5
nodes/setup/script_timing.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"duration_ms": 24,
|
||||
"exit_code": 128,
|
||||
"timed_out": false
|
||||
}
|
||||
6
nodes/setup/status.json
Normal file
6
nodes/setup/status.json
Normal file
|
|
@ -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:08:36.110826+00:00"
|
||||
}
|
||||
6
nodes/start/status.json
Normal file
6
nodes/start/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": null,
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-16T12:08:36.084977+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue