#29694 closed Bug (fixed)
QuerySet.values_list() combined with .extra() may produce wrong .union()
Reported by: | master | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 2.1 |
Severity: | Normal | Keywords: | union values_list |
Cc: | Mariusz Felisiak | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Change History (9)
comment:1 by , 6 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:2 by , 6 years ago
Description: | modified (diff) |
---|---|
Resolution: | duplicate |
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Since a fix for that ticket has been released, it's appropriate to open a new ticket. It seems that the test that was added in the patch doesn't precisely reflect the steps to reproduce.
comment:3 by , 6 years ago
(I wasn't sure and I don't mind to reopen a previous ticket, but my reasoning to open a new ticket was exactly the one of Tim).
So why didn't the test case (test_union_with_two_annotated_values_list) fail after the changes of #29286? Because it is written with annotate() instead of extra(). To reproduce the steps of the demo code of #29229, replace:
qs1 = Number.objects.filter(num=1).annotate(count=Value(0, IntegerField()),).values_list('num', 'count')
by:
qs1 = Number.objects.filter(num=1).extra(select={'count': 0}).values_list('num', 'count')
Note: I just discovered that the usage of extra() is discouraged, and I will consider a replacement, but not as a priority.
My suggestion for a fix in django\db\models\sql\compiler.py\get_combinator_sql(), to work both with extra and annotate, is something of this kind:
if (not compiler.query.values_select and not compiler.query.annotations and (self.query.values_select or self.query.annotation_select or self.query.extra_select)): compiler.query.set_values(tuple(self.query.values_select) + tuple(self.query.annotation_select) + tuple(self.query.extra_select) )
I'm not sure if we need the additional condition "and not compiler.query.<extra>", as I don't know what <extra> would be amongst: "extra", "extra_select" and "_extra".
comment:4 by , 6 years ago
Severity: | Release blocker → Normal |
---|---|
Summary: | QuerySet.values_list() combined with .extra() or .annotate() may produce wrong .union() → QuerySet.values_list() combined with .extra() may produce wrong .union() |
Since this was a regression, I'm open to fixing it on master and perhaps stable/2.1.x, however, be aware that we're no longer fixing bugs with QuerySet.extra()
per discussion on django-developers.
comment:5 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Version: | 1.11 → 2.1 |
comment:7 by , 6 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Please don't deliberately open duplicate tickets.
If there is still an issue on #29229, please comment there.
See the test case introduced in a0c03c62a8ac586e5be5b21393c925afa581efaf. Can you adjust this to demonstrate the issue you are still seeing? If so, we can re-open #29229 and address this.
(Either way we need some information to go on as to how to replicate the issue you are seeing. As far as we were aware the issue in the original description was already resolved...)