[Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
               Reporter:  shulcsm    |          Owner:  nobody
                   Type:  Bug        |         Status:  new
              Component:  Database   |        Version:  2.1
  layer (models, ORM)                |
               Severity:  Normal     |       Keywords:
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 {{{
 class Document(models.Model):
     pass

 class Picking(Document):
     document_ptr = models.OneToOneField(Document,
 on_delete=models.CASCADE, parent_link=True, related_name='+')
     origin = models.OneToOneField(Document, related_name='picking',
 on_delete=models.PROTECT)
 }}}

 produces django.core.exceptions.ImproperlyConfigured: Add parent_link=True
 to appname.Picking.origin.

 {{{
 class Picking(Document):
     origin = models.OneToOneField(Document, related_name='picking',
 on_delete=models.PROTECT)
     document_ptr = models.OneToOneField(Document,
 on_delete=models.CASCADE, parent_link=True, related_name='+')
 }}}
 Works

 First issue is that order seems to matter?
 Even if ordering is required "by design"(It shouldn't be we have explicit
 parent_link marker) shouldn't it look from top to bottom like it does with
 managers and other things?

--
Ticket URL: <https://code.djangoproject.com/ticket/29998>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.ae645d239f76f7242d8da2b03142c60f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (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
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * stage:  Unreviewed => Accepted


Comment:

 That seems to be a bug, managed to reproduce.

 Does the error go away if you add `primary_key=True` to `document_ptr`
 like I assume you wanted to do? This makes me realized that the
 [https://docs.djangoproject.com/en/2.1/topics/db/models/#multi-table-
 inheritance MTI documentation] is not completely correcy,
 [https://github.com/django/django/blob/7d1123e5ada60963ba3c708a8932e57342278706/django/db/models/options.py#L225-L244
 the automatically added `place_ptr` field end up with `primary_key=True`].

 Not sure why we're not checking `and field.remote_field.parent_link` on
 [https://github.com/django/django/blob/7d1123e5ada60963ba3c708a8932e57342278706/django/db/models/base.py#L183
 parent links connection].

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:1>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.196991a5d39e721b4f10c8eddda0eabd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (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 Mārtiņš Šulcs):

 Replying to [comment:1 Simon Charette]:

 It does go away primary_key.
 Why is parent_link even necessary in that case? Having pk OneToOne with to
 MTI child should imply it's parent link.

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:2>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.7328a62c77bc3722529841f07b520a9f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (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 Mārtiņš Šulcs):

 Replying to [comment:2 Mārtiņš Šulcs]:
 > Replying to [comment:1 Simon Charette]:
 >
 > It does go away primary_key.
 > Why is parent_link even necessary in that case? Having pk OneToOne with
 to MTI child should imply it's parent link.
 I have an update. The warning goes away with primary_key, but model itself
 is still broken(complains about document_ptr_id not populated) unless
 field order is correct.

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:3>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.9bcb30dd35a4023b7f3f2cb01bd4e62d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (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 Ben Beecher):

 Been able to replicate this bug with a simpler case:

 {{{
 class Document(models.Model):
     pass

 class Picking(Document):
     some_unrelated_document = models.OneToOneField(Document,
 related_name='something', on_delete=models.PROTECT)
 }}}

 Produces the same error against some_unrelated_document.

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:4>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.f80d87e42c96d75a01bd0ff2a90703fa%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (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
-------------------------------------+-------------------------------------
Changes (by Can Sarıgöl):

 * cc: Can Sarıgöl (added)


Comment:

 Hi, Could this approach be appropriate? {{{parent_links}}}'s params can be
 base and related class instance therefore just last sample columns are
 added into parent_links.
 We can extend key logic with related_name, e.g ('app', 'document',
 'picking').
 Or using this method, we can always ensure that the {{{parent_link=True}}}
 field is guaranteed into {{{self.parents}}}.


 {{{
 +++ b/django/db/models/base.py
 @@ -196,10 +196,11 @@ class ModelBase(type):
              if base != new_class and not base._meta.abstract:
                  continue
              # Locate OneToOneField instances.
 -            for field in base._meta.local_fields:
 -                if isinstance(field, OneToOneField):
 -                    related = resolve_relation(new_class,
 field.remote_field.model)
 -                    parent_links[make_model_tuple(related)] = field
 +            fields = [field for field in base._meta.local_fields if
 isinstance(field, OneToOneField)]
 +            for field in sorted(fields, key=lambda x:
 x.remote_field.parent_link, reverse=True):
 +                related_key =
 make_model_tuple(resolve_relation(new_class, field.remote_field.model))
 +                if related_key not in parent_links:
 +                    parent_links[related_key] = field

          # Track fields inherited from base models.
          inherited_attributes = set()
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:5>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.47b8edf7ef3a0f4b8c3f95fc6db9df20%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  Can
                                     |  Sarıgöl
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Can Sarıgöl):

 * owner:  nobody => Can Sarıgöl
 * status:  new => assigned
 * has_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:6>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.2cd26e35e1a45d8d6e203d01f9c43fea%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29998: pk setup for MTI to parent get confused by multiple OneToOne references.

Django
In reply to this post by Django
#29998: pk setup for MTI to parent get confused by multiple OneToOne references.
-------------------------------------+-------------------------------------
     Reporter:  Mārtiņš Šulcs        |                    Owner:  Can
                                     |  Sarıgöl
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by brianmaissy):

 What's the status on this? I'm encountering the same issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/29998#comment:7>
Django <https://code.djangoproject.com/>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.66c0ef37365ab5dafc102e4ba67cc3ff%40djangoproject.com.