Ticket 36472: Addressed issues with create() and save() on a model with GeneratedField(primary_key=True) #19580
Conversation
shangxiao
force-pushed
the
ticket-36472
branch
4 times, most recently
from
4e28820 to
f82c8ac
Compare
django/db/models/base.py
Outdated
| return getattr(self, meta.pk.attname) | ||
| try: | ||
| return getattr(self, meta.pk.attname) | ||
| except AttributeError: | ||
| return None |
There was a problem hiding this comment.
This may need to be removed and dealt with further up the stack... for databases that don't support RETURNING like mysql then a generated pk shouldn't return None but instead continue to raise AttributeError.
| if not instance._is_pk_set(): | ||
| if self.field.primary_key or not instance._is_pk_set(): |
There was a problem hiding this comment.
Actually we should be able to write a simple test for this, we should get an AttributeError instead of infinite recursion:
foo = Foo(value=1)
with self.assertRaises(AttributeError):
foo.generated_value
There was a problem hiding this comment.
I just realised that self.field.primary_key will not work if it's part of a composite key, which may need to be addressed separately in PR #19569
shangxiao
force-pushed
the
ticket-36472
branch
2 times, most recently
from
d8e0f24 to
731ab45
Compare
…the field itself is part of the pk.
…lt for pks that are deferred.
shangxiao
changed the title
|
buildbot, test on oracle. |
| setattr(self, meta.pk.attname, pk_val) | ||
| pk_set = self._is_pk_set(meta) | ||
| # if pk is deferred then this will raise an AttributeError | ||
| except AttributeError: |
There was a problem hiding this comment.
If this is the right approach then we should use a custom exception class so as not mask bugs:
class ModelAttributeError(AttributeError):
pass
Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com>
Trac ticket number
ticket-36472
Branch description
Looking at some possible fixes to see whether supporting GeneratedField(primary_key=True) can be elegantly patched.
PR broken into a couple of sensible commits:
Nonethatget_ancestor_link()may indeed return_is_pk_set()->_get_pk_val()-> back to deferred attribute again) if the field itself is a pkcreate()) UseNoneifAttributeErrorfromDeferredAttributesave()-- this one may actually be a separate issue as this was more of a why empty string for an integer field??)_get_default()returns empty string ("") as the default if none of the other conditions are satisfied, this will break a non-string column. Not sure why deferring to_get_default()is used ifNOT_PROVIDEDis set 🤔Checklist
mainbranch.I have attached screenshots in both light and dark modes for any UI changes.