[Django] #18081: Proxy model foreign keys not created properly by syncdb
Django
----------------------------------------------+--------------------
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Add this test to modeltests/proxy_models/tests.py:
{{{
class TransactionalProxyModelTests(TransactionTestCase):
def test_proxy_fk(self):
"""
Test that the DB contains proper foreign keys for proxy model
references.
"""
@transaction.commit_on_success
def create_failing_pk():
t = TrackerUser.objects.create(status='bar')
Improvement.objects.create(summary='foof', version='foof',
reporter_id=1, associated_bug_id=1,
assignee=t)
self.assertRaises(IntegrityError, create_failing_pk)
}}}
On MySQL this does not fail when using InnoDB. It should fail, as both
reporter and associated bug with ID=1 are missing.
The reason for this is that in django/db/backends/creation.py,
sql_for_pending_references() is a check:
{{{ if not model._meta.managed or model._meta.proxy: skip creation }}}
Now, that is incorrect: the model in question is the model we are
_referring_, not the model from where the PK is from. So, under MySQL
foreign keys to proxy models do not get enforced in the DB.
In addition the same bug is there for other databases, too. However, this
one does not show under PostgreSQL as the foreign key is created inline.
However, were the order of the models different in models.py also
PostgreSQL would fail to create the foreign key.
The fix is luckily very simple: just remove the "or proxy" part. The
managed part is correct: the referenced model can be a view for example,
in which case referencing it would be a failure.
--
Ticket URL: <https://code.djangoproject.com/ticket/18081>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:1>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: charette.s@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:2>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Two years later, I guess the branch probably doesn't merge cleanly.
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:3>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I am not sure if this is still an issue with the `SchemaEditor` backends.
I updated Anssi's test (attached) and it passes at
28cb272a7279e6dfc4d5c53838ebf7343c3e66b5, however, the
`SchemaEditor._model_indexes_sql()` method still
has[https://github.com/django/django/blob/244404227e8a1c5e241658ef0df789a28ed3bbc6/django/db/backends/base/schema.py#L852-L853
a condition to skip proxy models].
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:3>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "18081-test.diff" added.
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I don't see why that line in the `SchemaEditor` should be wrong. Proxy
models don't have any database representation on their own
(https://docs.djangoproject.com/en/1.8/topics/db/models/#proxy-models) and
thus don't have any indices. Without digging into the issue too deep, this
seems more like a bug in the assignment of related objects to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:4>
Django
proxy model.
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* cc: charette.s@… (removed)
* cc: charettes (added)
Comment:
This is not an issue anymore since calls to
[https://github.com/django/django/blob/244404227e8a1c5e241658ef0df789a28ed3bbc6/django/db/backends/base/schema.py#L872-L888
_create_fk_sql] are not conditional to ` not
field.target_field.model._meta.proxy`
([https://github.com/django/django/blob/244404227e8a1c5e241658ef0df789a28ed3bbc6/django/db/backends/base/schema.py#L251-L252
create_model],
[https://github.com/django/django/blob/244404227e8a1c5e241658ef0df789a28ed3bbc6/django/db/backends/base/schema.py#L411-L412
add_field],
[https://github.com/django/django/blob/244404227e8a1c5e241658ef0df789a28ed3bbc6/django/db/backends/base/schema.py#L714-L722
alter_field]).
I think a more appropriate regression test could live in `test_operations`
and issue an `AddField` of a foreign key pointing to `ProxyPony` and make
sure the constraint exists. Naturally the test should be
`skipUnlessDBFeature('supports_foreign_keys')`.
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:5>
Django
proxy model.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
[https://github.com/django/django/pull/5120 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:6>
Django
proxy model.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8e8c0792c07f6dfbee5115d29911191adee04f2e" 8e8c0792]:
{{{
#!CommitTicketReference repository=""
revision="8e8c0792c07f6dfbee5115d29911191adee04f2e"
Refs #18081 -- Asserted db constraints are created for fk to proxy models.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:7>
Django
proxy model.
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.4
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/18081#comment:8>