Opened 8 years ago

Closed 8 years ago

#7136 closed (fixed)

when inheriting models, accessing related subclass instances can produce spurious relations depending on order

Reported by: Andy MacKinlay Owned by: socrates
Component: Core (Other) Version: master
Severity: Keywords: model inheritance
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you are using the new multi-table inheritance feature, and have two or more subclasses (eg SubClassA and SubClassB) which inherits from a baseclass, and retrieve a base class instance baseinstance from the DB, the expected behaviour if baseinstance only has a corresponding SubClassA instance should be to throw a DoesNotExist exception when accessing baseinstance.subclassb. However, if you have already accessed baseinstance.subclassa, baseinstance.subclassb will point to the corresponding SubClassA instance instead of raising the exception.

The attached models.py includes a test case that documents this.

Attachments (5)

models.py (1.4 KB) - added by Andy MacKinlay 8 years ago.
model file with included doctest to replicate the issue
shell_results.py (4.6 KB) - added by socrates 8 years ago.
This file is to easily reproduce the shell results and see the situation before and after the patch
patch_for_rev7513.diff (4.5 KB) - added by socrates 8 years ago.
stricter type checking
patch_for_rev7513_alternative.diff (4.0 KB) - added by socrates 8 years ago.
an alternative pach. in stead of stricter type checking it avoids name collisions by changing the cache_name.
shell_results_rev7513.py (5.4 KB) - added by socrates 8 years ago.
This file is to easily reproduce the shell results and see the situation before and after the patch (this version includes the models)

Download all attachments as: .zip

Change History (12)

Changed 8 years ago by Andy MacKinlay

Attachment: models.py added

model file with included doctest to replicate the issue

comment:1 Changed 8 years ago by socrates

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to socrates
Patch needs improvement: unset

comment:2 Changed 8 years ago by socrates

Status: newassigned

Changed 8 years ago by socrates

Attachment: shell_results.py added

This file is to easily reproduce the shell results and see the situation before and after the patch

comment:3 Changed 8 years ago by socrates

Has patch: set

As it tuns out the bug was even a bit more extensive: not only accessing a non existing subclass goes wrong. Also if the subclass does exist then in some special cases (multiple inheritance) the wrong object is returned (see shell_result.py). The patch (patch_for_rev7513.diff) contains a doctest that describes multiple model inheritance.

comment:4 Changed 8 years ago by socrates

The patch fixes the problem but I am not certain if this is the right way to go about it. The problem was initially caused because of name collisions in the SingleRelatedObjectDescriptor.cache_name. Maybe a different naming contvention for the cache_name is the right way to fix it.

Changed 8 years ago by socrates

Attachment: patch_for_rev7513.diff added

stricter type checking

Changed 8 years ago by socrates

an alternative pach. in stead of stricter type checking it avoids name collisions by changing the cache_name.

Changed 8 years ago by socrates

Attachment: shell_results_rev7513.py added

This file is to easily reproduce the shell results and see the situation before and after the patch (this version includes the models)

comment:5 Changed 8 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

I'm marking this as accepted, here's another test I put together that shows the problem: http://dpaste.com/hold/48236/

comment:6 Changed 8 years ago by socrates

It seems that this bug has already been fixed somewhere else so this ticket can be closed as duplicate.

comment:7 Changed 8 years ago by Alex Gaynor

Resolution: fixed
Status: assignedclosed

This was indeed fixed(don't have the revision handy).

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