Opened 6 years ago

Closed 6 years ago

#15854 closed Bug (wontfix)

Combining two querysets generated by "extra" causes duplication in extra_tables

Reported by: cwu Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: db sql union extra
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


I am using Django v1.2.3 with Oracle DB backend. The code below demo the possible bug I found:

#I have two models
tbl1 = model_1._meta.db_table
tbl2 = model_2._meta.db_table
#let's say
qs1 = model_1.objects.extra(tables=[tbl2], where=["%s.field1=%s.field2" % (tbl1, tbl2)])        
qs2 = model_1.objects.extra(tables=[tbl2], where=["%s.field3=%s.field4" % (tbl1, tbl2)])
qs3 = qs1 | qs2

Both qs1 and qs2 are built from "extra" by including the same tbl2. Then when I took the union of two (qs3), I got Oracle error "ORA-00918: column ambiguously defined". I dug into the django db code, and realized the problem was because qs3.query.extra_tables contains duplicated "tbl2" after combining (taking union) "qs1" and "qs2", which further causes duplicated tbl2 in from clause:

In [44]: qs3.query.extra_tables
Out[44]: ('tbl2', 'tbl2')

In [43]: qs3._as_sql(connection)
('SELECT TBL1."ID" FROM "TBL1" , "TBL2" , "TBL2" WHERE (tbl1.field1=tbl2.field2 OR tbl1.field3=tbl2.field4)',

I am not sure if this will cause problem in other DB types, but clearly Oracle does not like it. I ended up fixing it by removing duplication after taking union, but before the query was executed.

qs3.query.extra_tables = tuple(set(qs3.query.extra_tables))

To actual fix this issue from Django side, I guess uniquness-checking can be either in




or may be both, just to make sure the generated "from" clause doesn't contain duplicated tables.

I double-checked that I can reproduce this problem in either v1.2.5 or v1.3 .

Change History (1)

comment:1 Changed 6 years ago by Jacob

Resolution: wontfix
Status: newclosed

Once you start using extra you're basically telling Django that you're now going to be responsible for constructing the query. That is, extra can't possibly check for all the various ways it might break your SQL query, so we basically assume that if you use extra you know what you're doing and can do things like this by hand in your code.

Thus I'm marking this won't fix: we have no real intrest in making extra smarter since that's a slippery slope of never-ending edge cases.

Note: See TracTickets for help on using tickets.
Back to Top