Opened 9 years ago

Closed 6 years ago

#7258 closed Bug (fixed)

connection in WhereNode does not come down from Query

Reported by: Koen Biermans <koen.biermans@…> Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The connection used in WhereNodes is not the one in Query, but an import from django.db.

I am attaching a patch that adds the connection via the initialisation of the wherenode. The catch is I had to fix the deepcopying: the connection should be copied as a reference.

I do not know whether this will break the ability to pickle the query, an extra getstate/setstate may be required here.

Attachments (1)

connection_in_sqlqueries.diff (8.2 KB) - added by Koen Biermans <koen.biermans@…> 9 years ago.
patch to pass connection from Query into WhereNode

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by Koen Biermans <koen.biermans@…>

patch to pass connection from Query into WhereNode

comment:1 Changed 8 years ago by George Vilches

Keywords: qsrf-cleanup added

comment:2 Changed 8 years ago by jbronn

Resolution: invalid
Status: newclosed

It's OK that the connection does not come from Query. This is because the connection is not used to run any queries on the database, but rather to get general properties specific to the database backend. Notice that every use of connection is to get operators or operations specific to backend. Unless there's some buggy behavior I'm missing, I think this is what Malcolm intended.

comment:3 Changed 8 years ago by Koen Biermans <koen.biermans@…>

Resolution: invalid
Status: closedreopened

I may be out of line here, but reopening anyway.

This was obviously intended in the light of multiple database usage: if you use different database engines, you will need different options used to create the SQL. Somehow the WhereNode needs to know which options to use. Since Query already has a handle on the connection object, passing the connection object down seemed like a good approach.

I think this deserves more discussion.

comment:4 Changed 8 years ago by Malcolm Tredinnick

Keywords: qsrf-cleanup WhereNode removed
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Please don't reopen tickets that have been marked as wontfix. That's clearly documented (particularly so many months later).

That said, we will do something about this, although not with this patch; it's pretty ugly.

For now, live with the reality that Django doesn't support multiple database connections. Well it does, very unofficially, but don't try to mix and match wildly different backends. That's impossible for other reasons already (for example BaseQuery vs. Oracle Query) and I'm not really interested in piecemeal solutions to that problem, since it's a larger thing. In this case, though, we might be able to use a local connection object if it's not going to be too intrusive or performance intensive (we do a *lot* of cloning, which is why WhereNode has no init` method or much in the way of deep structure beyond the tree nodes. I want to keep it that way.)

comment:5 Changed 6 years ago by Luke Plant

Resolution: fixed
Severity: Normal
Status: reopenedclosed
Type: Bug

I might be wrong, but I think this is fixed now.

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