#34884: Half bug/half enhancement : inconsistent behavior of get_or_create()
regarding related attributes cache
-------------------------------------+-------------------------------------
Reporter: Laurent Lyaudet | Owner: nobody
Type: Uncategorized | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: ORM get_or_create | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Hi Laurent,
To echo what Natalian said, please avoid comments like "in case some
bikeshedding occurs that puts to trash coherency and efficiency in yet
another open source software where some people against free software have
influence". It adds nothing helpful. Assuming bad faith on the part of the
Django community is no way to introduce yourself and get people to want to
work with you. Thank you.
As for the issue, I think it's a bit of a niche case. I mocked up a patch
using your proposal. I'm not sure that the extra complexity is worthwhile,
as well as the performance penalty of iterating through the `kwargs`
(which is unnecessary when a foreign key isn't present). The patch may
need consideration for other relations like `GenericForeignKey`. I'm not
sure offhand. I wonder if `update_or_create()` has a similar
inconsistency. I'm afraid that trying to make the ORM smarter about this
might open a can of worms.
Cheers!
{{{ #!diff
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 1125302933..818d31faac 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -939,11 +939,20 @@ class QuerySet(AltersData):
Return a tuple of (object, created), where created is a boolean
specifying whether an object was created.
"""
+ from django.db.models.fields.related import ForeignKey
# The get() needs to be targeted at the write database in order
# to avoid potential transaction consistency problems.
self._for_write = True
try:
- return self.get(**kwargs), False
+ obj, created = self.get(**kwargs), False
+ for key, value in kwargs.items():
+ try:
+ if isinstance(obj._meta.get_field(key), ForeignKey):
+ # isinstance handles OneToOneField also.
+ setattr(obj, key, value)
+ except exceptions.FieldDoesNotExist:
+ pass
+ return obj, created
except self.model.DoesNotExist:
params = self._extract_model_params(defaults, **kwargs)
# Try to create an object using passed params.
@@ -953,6 +962,7 @@ class QuerySet(AltersData):
return self.create(**params), True
except IntegrityError:
try:
+ # Another get() case that would need to be updated.
return self.get(**kwargs), False
except self.model.DoesNotExist:
pass
diff --git a/tests/get_or_create/tests.py b/tests/get_or_create/tests.py
index 0b56d6b1a2..4aac3ff580 100644
--- a/tests/get_or_create/tests.py
+++ b/tests/get_or_create/tests.py
@@ -129,8 +129,19 @@ class GetOrCreateTests(TestCase):
self.assertEqual(book.authors.count(), 2)
# Try creating a book through an author.
- _, created = ed.books.get_or_create(name="Ed's Recipes",
publisher=p)
+ eds_recipes, created = ed.books.get_or_create(name="Ed's
Recipes", publisher=p)
self.assertTrue(created)
+ # In the created=True case, the publisher is set on the new
object.
+ with self.assertNumQueries(0):
+ self.assertEqual(eds_recipes.publisher.name, 'Acme
Publishing')
+
+ # In the case of created=False, the publisher isn't set on the
object.
+ eds_recipes, created = ed.books.get_or_create(name="Ed's
Recipes", publisher=p)
+ self.assertFalse(created)
+ # This assertion fails due to the query required to fetch the
+ # publisher, demonstrating #34884.
+ with self.assertNumQueries(0):
+ self.assertEqual(eds_recipes.publisher.name, 'Acme
Publishing')
# Now Ed has two Books, Fred just one.
self.assertEqual(ed.books.count(), 2)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34884#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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018af33e75c0-a460775a-c7d3-49e9-8a8a-795349a1877e-000000%40eu-central-1.amazonses.com.
