Opened 17 years ago
Closed 15 years ago
#7258 closed Bug (fixed)
connection in WhereNode does not come down from Query
| Reported by: | Owned by: | nobody | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
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)
Change History (6)
by , 17 years ago
| Attachment: | connection_in_sqlqueries.diff added |
|---|
comment:1 by , 17 years ago
| Keywords: | qsrf-cleanup added |
|---|
comment:2 by , 17 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → 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 by , 17 years ago
| Resolution: | invalid |
|---|---|
| Status: | closed → 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 by , 17 years ago
| Keywords: | qsrf-cleanup WhereNode removed |
|---|---|
| Patch needs improvement: | set |
| Triage Stage: | Unreviewed → 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 by , 15 years ago
| Resolution: | → fixed |
|---|---|
| Severity: | → Normal |
| Status: | reopened → closed |
| Type: | → Bug |
I might be wrong, but I think this is fixed now.
patch to pass connection from Query into WhereNode