Re: [Django] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
-+-
 Reporter:  Afief|Owner:  aaugustin
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.1
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  1
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Aymeric Augustin ):

 In [changeset:"f855058c35db9d35ba7f310d8f9db6f4533b7424"]:
 {{{
 #!CommitTicketReference repository=""
 revision="f855058c35db9d35ba7f310d8f9db6f4533b7424"
 [1.6.x] Fixed #11811 -- Data-loss bug in queryset.update.

 It's now forbidden to call queryset.update(field=instance) when instance
 hasn't been saved to the database ie. instance.pk is None.

 Conflicts:
 tests/queries/tests.py

 Backport of b4cd8169 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/063.4b8889535e1e5cfaab85aba2366e31d0%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
-+-
 Reporter:  Afief|Owner:  aaugustin
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.1
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  1
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by aaugustin):

 In fact, this may be fixed independently from #10811, which turned out to
 be extremely hard.

 While I can imagine use cases for the behavior #10811 is trying to
 prevent, I don't see any for this one. It's clearly a data-loss bug.

-- 
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/063.1539c04b003e4e79598ca96336107ca2%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
-+-
 Reporter:  Afief|Owner:  aaugustin
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.1
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  1
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Aymeric Augustin ):

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


Comment:

 In [changeset:"b4cd8169de604943f8aaee3666282c95e650e5f4"]:
 {{{
 #!CommitTicketReference repository=""
 revision="b4cd8169de604943f8aaee3666282c95e650e5f4"
 Fixed #11811 -- Data-loss bug in queryset.update.

 It's now forbidden to call queryset.update(field=instance) when instance
 hasn't been saved to the database ie. instance.pk is None.
 }}}

-- 
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/063.09e709ade5fd6ae8fbf61e7ee442368d%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
-+-
 Reporter:  Afief|Owner:  aaugustin
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.1
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  1
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by aaugustin):

 * owner:  ashearer => aaugustin


-- 
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.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
-+-
 Reporter:  Afief|Owner:  ashearer
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.1
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  1
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by aaugustin):

 Related to #10811.

-- 
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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.3 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  1   
Needs_docs:  1 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Changes (by russellm):

  * milestone:  1.2 => 1.3

Comment:

 Not critical for 1.2

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  1   
Needs_docs:  1 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Changes (by ashearer):

  * needs_docs:  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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  1   
Needs_docs:  0 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Comment (by ashearer):

 Here are some examples to make this more concrete.

 {{{
 # Nullable foreign key field, without patch, previous surprising behavior:
 >>> s = Site(name='My new site')
 >>> f = Flatpage(id=1, text='I am being published on that site')
 >>> f.site = s  # set a site for the Flatpage
 >>> s.save()# (site is saved now, but doesn't change
 outcome)
 >>> f.site  # site value is still there, looks good
 
 }}}

 Old behavior from that point:
 {{{
 >>> f.save()# save it to DB
 >>> f = Flatpage.objects.get(id=1)  # read it back
 >>> print f.site# ...and the Flatpage's site value disappears!
 None
 }}}

 Patched behavior from that point:
 {{{
 >>> f.save()# will now raise an exception about the
 anomaly
 Traceback (most recent call last):
 ...
 ValueError: Cannot save a foreign key referring to the instance "" into the field "Flatpage.site", because the key value for the
 instance was not available at the time of assignment to the field. (For
 autonumbered keys, the related instance may need to be saved prior to the
 assignment.)
 }}}


 The behavior for assignment to non-null fields with the rev-5 patch is the
 same as before:
 {{{
 # Test a non-null field. Behavior is unchanged.
 >>> d = Department(name='An Unsaved Department')
 >>> w = Worker(id=1, name='Milton Waddams', department=d)
 >>> w.save()
 Traceback (most recent call last):
 ...
 IntegrityError: model_regress_worker.department_id may not be NULL
 or
 Warning: Column 'department_id' cannot be null
 or
 other DB-specific error
 }}}

 However, the batch queryset.update() method raises a new ValueError
 exception regardless of nullability:
 {{{
 >>> d1 = Department(id=1, name='A Real Department')
 >>> d1.save()
 >>> d2 = Department(name='An Unsaved Department')
 >>> Worker.objects.create(id=2, name='Milton Waddams', department=d1)
 
 >>> Worker.objects.filter(id=2).update(department=d2)
 Traceback (most recent call last):
 ...
 ValueError: Cannot assign the instance "" to a foreign key field, because its primary key is not
 available. (For autonumbered keys, the instance would need to be saved
 first.)
 }}}

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  1   
Needs_docs:  0 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Changes (by ashearer):

  * has_patch:  0 => 1

Comment:

 The updated patch (rev-5) doesn't raise exceptions for model instances
 that are never saved anyway, which fixes the multi-DB test failures.

 The one remaining design issue is a backward compatibility vs. consistency
 tradeoff. The patch errs on the side of backward compatibility: if the
 foreign key field is declared non-null, assignment works as before, and
 lets the database raise its own integrity error instead of the new
 ValueError exception with a detailed error message. The queryset.update()
 patch can't tell how the field is declared, though, so it always raises
 ValueError. Raising ValueError in all cases would be more consistent and
 probably more helpful, but it could break compatibility for code that's
 already expecting to catch a particular integrity error.

 Also, the patch could attempt to be smart. When it detects that the
 foreign key value is missing but there is a cached instance, it could fill
 the key in from that instance, in case the instance was saved after the
 assignment. But that's starting to get magical, so it doesn't do that.

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  0   
Needs_docs:  0 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Comment (by ashearer):

 I shouldn't have said "passing an unsaved model acts the same as passing
 None". That's true only for the ForeignKeyField as saved in the DB. (It
 doesn't matter whether the unsaved foreign model itself is saved after the
 assignment.) In memory, the attribute still points to the unsaved model
 instance.

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  0   
Needs_docs:  0 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Comment (by ashearer):

 The same surprising behavior occurs whether you assign to the ForeignKey
 field by attribute assignment, a model initializer,
 model.objects.create(), or queryset.update(). In any of the cases, passing
 an unsaved model acts the same as passing None. (That's assuming no manual
 pk assignment.) The first three use a different code path than the last.

 I'm attaching a strawman patch that will raise a ValueError with a helpful
 message in all four cases. The docs had left the behavior undefined.

 However, the patch needs further work:
  1. Raising an exception at assignment time may be a bad idea, even though
 the inconsistency between my_fk_field (an object) and my_fk_field_id
 (None) starts then. Everything would have worked OK while the same object
 remained in memory; only the copy saved in the DB will have no link at all
 to the other instance. Checking at save time may be better.
  2. To match the scope of the ticket, I'm only handling assignment for
 non-null fields. Non-nullable fields already raise an exception, though it
 occurs later (on save), and the error message is less specific as a
 result. But the queryset.update() code path now always throws the new
 exception regardless of whether the field is nullable, because field
 information isn't available to it.
  3. Two tests of multi-DB support now fail, because they make the same
 mistake the new exception is designed to guard against. Either putting
 explicit IDs in the tests or changing the timing of the exceptions as per
 point 1 may help. The rest of the test suite has no additional failures.

 The patch includes tests but not docs. I'll attach a docs patch separately
 once the implementation settles down.

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  ashearer
Status:  assigned  | Milestone:  1.2 
 Component:  Database layer (models, ORM)  |   Version:  1.1 
Resolution:|  Keywords:  
 Stage:  Accepted  | Has_patch:  0   
Needs_docs:  0 |   Needs_tests:  0   
Needs_better_patch:  0 |  
---+
Changes (by ashearer):

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

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
---+
  Reporter:  Afief | Owner:  nobody
Status:  new   | Milestone:  1.2   
 Component:  Database layer (models, ORM)  |   Version:  1.1   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  0 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by russellm):

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

-- 
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-upda...@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] #11811: No error raised on update(foreignkey=unsavedobject) on nullable fk

[Django]

#11811: No error raised on update(foreignkey=unsavedobject) on nullable fk
--+-
 Reporter:  Afief |   Owner:  nobody
   Status:  new   |   Milestone:
Component:  Database layer (models, ORM)  | Version:  1.1   
 Keywords:|   Stage:  Unreviewed
Has_patch:  0 |  
--+-
 Found this while coding at 2AM:
 When there is a field that can be NULL, and you do something like this:
 obj1=Obj1(...)
 obj2=Obj2.objects.all().update(foreignobj=obj1)

 django will assign the fields to NULL.

 I'm not sure if this behaviour is made to be so by design, but it seems to
 me that if someone passes an unsaved object instead of None they made a
 mistake and django should raise an exception.

-- 
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
-~--~~~~--~~--~--~---