Opened 11 years ago
Closed 9 years ago
#21554 closed Bug (fixed)
incorrect SQL generated when using multiple inheritance
Reported by: | Matt | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | multiple-inheritance |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Our application makes use of multiple-inheritance and we've just come across an interesting bug. There is a test case attached and it fails against the latest 1.4, 1.5, 1.6, and master versions. Though the reason changes between versions 1.5 and 1.6.
The models:
class Person(models.Model): name = models.CharField(max_length=50) class Politician(models.Model): politician_id = models.AutoField(primary_key=True) title = models.CharField(max_length=50) class Congressman(Person, Politician): state = models.CharField(max_length=2) class Senator(Congressman): pass
The statement Senator.objects.get(politician_id=1)
produces incorrect SQL for all versions, but is different between versions.
1.4.10 and 1.5.5 result in the error "DoesNotExist: Senator matching query does not exist." due to it trying to join demo_politician onto demo_senator via demo_senator.congressman_ptr_id instead of demo_politician.politician_id
SELECT "demo_person"."id", "demo_person"."name", T5."politician_id", T5."title", "demo_congressman"."politician_ptr_id", "demo_congressman"."person_ptr_id", "demo_congressman"."state", "demo_senator"."congressman_ptr_id" FROM "demo_senator" INNER JOIN "demo_congressman" ON ("demo_senator"."congressman_ptr_id" = "demo_congressman"."person_ptr_id") INNER JOIN "demo_person" ON ("demo_senator"."congressman_ptr_id" = "demo_person"."id") INNER JOIN "demo_politician" T5 ON ("demo_senator"."congressman_ptr_id" = T5."politician_id") WHERE "demo_congressman"."politician_ptr_id" = 1
1.6 and master results in the error "OperationalError: no such column: demo_congressman.politician_id". It incorrectly thinks that politician_id is on demo_congressman instead of demo_politician.
SELECT "demo_person"."id", "demo_person"."name", "demo_congressman"."politician_id", "demo_congressman"."title", "demo_congressman"."politician_ptr_id", "demo_congressman"."person_ptr_id", "demo_congressman"."state", "demo_senator"."congressman_ptr_id" FROM "demo_senator" INNER JOIN "demo_congressman" ON ( "demo_senator"."congressman_ptr_id" = "demo_congressman"."person_ptr_id" ) INNER JOIN "demo_person" ON ( "demo_congressman"."person_ptr_id" = "demo_person"."id" ) WHERE "demo_congressman"."politician_ptr_id" = 1
Attachments (1)
Change History (12)
by , 11 years ago
comment:3 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I haven't investigated but the report looks legitimate. Obviously a proper patch with tests will help get this fixed.
comment:4 by , 11 years ago
Has patch: | set |
---|
I packaged up mitchell's patch into a pull request here: https://github.com/django/django/pull/2034
I don't know enough about Django to know the implications of making that change to get_default_columns, but I guess that's why I don't have that commit bit. The test suite did pass after the change:
Ran 6295 tests in 325.761s OK (skipped=419, expected failures=8)
comment:5 by , 11 years ago
Patch needs improvement: | set |
---|
I added comments on PR. Apart from the minor suggested changes the patch looks RFC; full test suite passes on SQLite3 Py2.
comment:6 by , 11 years ago
Thanks for reviewing. I made the suggested changes. Tests continue to pass fully.
comment:7 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:8 by , 11 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Test needs to be extended to test that the title is correct. Currently the patch replaces all fields from non-primary classes with the ID. Also, the second base was not being joined to the query correctly.
Second pull request here: https://github.com/django/django/pull/2059
comment:9 by , 10 years ago
I left comments for improvement on PR. Please uncheck "Patch needs improvement" when you update it, thanks.
comment:11 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I guess this issue has since been fixed (unless the regression tests in the second pull request don't capture the issue). Please open a new ticket if issues remain, thanks!
I've also stumbled onto this one today.
I've made a gist of a rudimentary module with a quick and dirty monkey patch for Django 1.6.
I'm new to Django so don't feel comfortable submitting a proper patch.