Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  jenh
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:  sql constraint   |  checkin
  proxy  |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by timo):

 In aa830009de940e31711b4858501becd7f34a1972:

 Fixed #17519 -- Fixed missing SQL constraints to proxy models.

 Thanks thibaultj for the report, jenh for the patch,
 and charettes for the tests.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.39112e8b098fd4c0e7e71d91425139bd%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  jenh
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:  sql constraint   |  checkin
  proxy  |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Tim Graham ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"196cc875b26f266e8f8dfd5be9daa0b8f246b9cd"]:
 {{{
 #!CommitTicketReference repository=""
 revision="196cc875b26f266e8f8dfd5be9daa0b8f246b9cd"
 [1.6.x] Fixed #17519 -- Fixed missing SQL constraints to proxy models.

 Thanks thibaultj for the report, jenh for the patch,
 and charettes for the tests.

 Backport of aa830009de from master
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.1d95135fcd52cf05fba4d9ccef422098%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  jenh
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Ready for
 Keywords:  sql constraint   |  checkin
  proxy  |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by timo):

 * stage:  Accepted => Ready for checkin


Comment:

 Verified that the tests fail on MySQL before the fix and pass on all
 backends after (didn't try Oracle). I'm not confident enough to check this
 in myself, but am marking it as such so someone else can verify. Here's a
 [https://github.com/django/django/pull/1424 PR] that merges cleanly.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.9073a90a3164beb7f10b28fe0f3e9054%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  jenh
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql constraint   |  Needs documentation:  0
  proxy  |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by jenh):

 * status:  new => assigned
 * owner:  nobody => jenh


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql constraint   |  Needs documentation:  0
  proxy  |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by jenh):

 Is there anything else that I can do to progress this, or are we just
 needing a reviewer to mark as ready for checkin (or state why it isn't)?
 I'm not sure what if any documentation should be added.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql constraint   |  Needs documentation:  0
  proxy  |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by charettes):

 * keywords:  sql mysql postgresql constraint => sql constraint proxy
 * cc: charette.s@… (added)
 * version:  1.3 => master
 * needs_tests:  1 => 0


Comment:

 Added the missing tests.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  postgresql constraint  |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  0|
-+-
Changes (by jenh):

 * keywords:  sql mysql constraint => sql mysql postgresql constraint
 * has_patch:  0 => 1
 * needs_tests:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  constraint |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by jenh):

 After further investigation, it appears to be merely coincidence that in
 my cases the presence of related_name corresponds exactly with whether the
 constraint is created. Sorry for that red herring.

 The reason some of my !ForeignKeys get constraints is that they are
 handled in db/backends/creation.py
 !BaseDatabaseCreation.sql_for_inline_foreign_key_references, when the
 proxy model being referenced happens to be in the list of the app's models
 that have been processed at the point it reaches this non-proxy model. The
 SQL generator has no problem with the fact that the referenced model is a
 proxy model, since it uses field.rel.to._meta.db_table, which matches the
 parent model.

 The reason the other !ForeignKeys don't get constraints is that if the
 model containing the references is processed before the proxy models it is
 referencing, the references get deferred and are handled in
 !BaseDatabaseCreation.sql_for_pending_references. Crucially, this method
 operates on the model being referenced, and thus referenced proxy models
 fall afoul of its ditching of unmanaged and proxy models.

 Now, it seems to me that there is no reason for sql_for_pending_references
 to not do anything with proxy models. The constraints it creates use the
 underlying database table and are perfectly fine, and if there are no
 pending references to the proxy model, the method does nothing. However,
 being completely new to this part of Django's code, I may well be missing
 something important.

 I'm attaching a patch that just removes the requirement that a model not
 be a proxy model in order to create pending references to it. I don't have
 a test that this fixes the problem; if someone can give me some pointers
 on where that test should go and how it should be formulated, I'll do my
 best to create one. It does at least not cause any of the existing tests
 to fail (checked under PostgreSQL and SQLite (which doesn't use the code
 that's changed)).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  constraint |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by jenh):

 * cc: jenh (added)


Comment:

 I am getting the same behaviour with SQLite and PostgreSQL, only with a
 slight twist. If the ForeignKey is not given a related_name, the SQL will
 include a reference to the parent model. It is only when a related_name is
 present that the SQL will not include a reference. This strikes me as
 rather odd, but I haven't ever delved into the database code.

 As for the use of specifying the proxy model in the model definition, I
 see it as a matter of clarity. The ORM provides useful abstractions, and
 this is one - the reference really is, in the example, to a Father and not
 to a Parent, and the fact that this distinction does not exist at the
 database level is immaterial at the ORM level.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  constraint |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by thibaultj):

 I don't think this is directly related to the ForeignKey implementation.
 No matter how the python model and class inheritance is handled, data
 integrity must be respected on the db level.

 Consider the following exemple:

 {{{
 random_id = 666
 child = Child()
 child.name = 'random name
 child.parent_id = random_id
 child.save() # Throws IntegrityError
 child.father_id = random_id
 child.save() # No exception thrown
 }}}

 That means that you can have crappy data in db, which breaks the admin
 modules for those models.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  constraint |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by akaariai):

 * cc: anssi.kaariainen@… (added)


Comment:

 Sorry for the stupid question, but what is the point of allowing foreign
 keys to proxy models at all? In the DB the relation is to the base class
 anyways. The only point I can see is you want to return instances of the
 proxy class instead of the base class, but I hope that could be fixed by
 something like Parent.objects.as_model(Father). This would have uses
 beyond just `ForeignKeys`.

 My point is that is it worth fixing this by complicating the `ForeignKey`
 implementation? You don't get that much from the work after all. I haven't
 actually checked what it would need to fix this. One possibility is to
 just make the `ForeignKey` in reality to the base class and when
 constructing the related query set, just start from the Father model.

 If it is easy to fix the current implementation, by all means lets do
 that. But I am not too sure that is the case here...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
-+-
 Reporter:  thibaultj|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.3
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  sql mysql|  Needs documentation:  0
  constraint |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by anonymous):

 * needs_better_patch:   => 0
 * stage:  Unreviewed => Accepted
 * needs_tests:   => 0
 * needs_docs:   => 0


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

[Django] #17519: Missing SQL constraint to proxy models

[Django]

#17519: Missing SQL constraint to proxy models
--+
 Reporter:  thibaultj |  Owner:  nobody
 Type:  Bug   | Status:  new
Component:  Database layer (models, ORM)  |Version:  1.3
 Severity:  Normal|   Keywords:  sql mysql
 Triage Stage:  Unreviewed|  constraint
Easy pickings:  0 |  Has patch:  0
  |  UI/UX:  0
--+
 In a model class, when you add a ForeignKey to a proxy model, the sql
 reference will no be created in Mysql. This is a severe issue, since the
 integrity structure of the database cannot be ensured.

 Moreover, as stated in #16128, the django admin will not gather related
 proxy model objects when deleting something. This is a separate issue
 though, since we are talking of the sql generation here.


 {{{
 # file constraint_test/models.py
 class Parent(models.Model):
 name = models.CharField(max_length=25)

 class Father(Parent):
 class Meta:
 proxy = True

 def __unicode__(self):
 return "I'm your father, luke"

 class Child(models.Model):
 name = models.CharField(max_length=25)
 parent = models.ForeignKey(Parent, related_name='child_parent_set')
 father = models.ForeignKey(Father, related_name='child_father_set')
 }}}


 {{{
 python manage.py sql constaint_test
 }}}


 {{{
 BEGIN;
 CREATE TABLE `constraint_test_parent` (
 `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
 `name` varchar(25) NOT NULL
 )
 ;
 CREATE TABLE `constraint_test_child` (
 `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
 `name` varchar(25) NOT NULL,
 `parent_id` integer NOT NULL,
 `father_id` integer NOT NULL
 )
 ;
 ALTER TABLE `constraint_test_child` ADD CONSTRAINT
 `parent_id_refs_id_875b6ff2` FOREIGN KEY (`parent_id`) REFERENCES
 `constraint_test_parent` (`id`);
 -- Missing constraint to the father table
 COMMIT;
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.