Opened 7 years ago

Closed 4 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@…> 7 years ago.
patch to pass connection from Query into WhereNode

Download all attachments as: .zip

Change History (6)

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

patch to pass connection from Query into WhereNode

comment:1 Changed 7 years ago by gav

  • Keywords qsrf-cleanup added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by jbronn

  • Resolution set to invalid
  • Status changed from new to closed

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 6 years ago by Koen Biermans <koen.biermans@…>

  • Resolution invalid deleted
  • Status changed from closed to reopened

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 6 years ago by mtredinnick

  • Keywords qsrf-cleanup WhereNode removed
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 4 years ago by lukeplant

  • Resolution set to fixed
  • Severity set to Normal
  • Status changed from reopened to closed
  • Type set to Bug

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

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