RelatedManager.all().get_or_create() does not work

[Nick Pope]

When accessing the queryset for a related manager, .get_or_create() and .update_or_create() lose context of the related instance.

Calling on the related manager works as expected:

publisher.books.get_or_create(name="The Very Hungry Caterpillar")

This is the case that was fixed by #3121 and #23611.

But calling on the queryset causes an IntegrityError to be raised:

publisher.books.all().get_or_create(name="The Very Hungry Caterpillar")

This can be a subtle failure that is hard to understand, especially if publisher.books.all() is assigned to a variable earlier.

The challenge here is that the overridden methods on the manager set kwargs[self.field.name] = self.instance.

We'd need to be able to pass this information down to the queryset. It looks like this might already be available in _known_related_objects which is set by RelatedManager._apply_rel_filters(), so that would be a good starting point for investigation.

We would also need to check whether something needs to be done to select the correct database as RelatedManager.get_or_create() has handling for this:

            db = router.db_for_write(self.model, instance=self.instance)

If this is something we can't fix reliably, then we should update the admonition under ​.get_or_create() in the docs and probably update ​.update_or_create() to make this and other issues related to use through RelatedManager more clear.

[Nick Pope]

The following test can be added to GetOrCreateTests in tests/get_or_create/tests.py below test_get_or_create_on_related_manager:

    def test_get_or_create_on_related_queryset(self):
        p = Publisher.objects.create(name="Acme Publishing")
        # Create a book through the publisher.
        book, created = p.books.all().get_or_create(name="The Book of Ed & Fred")
        self.assertTrue(created)
        # The publisher should have one book.
        self.assertEqual(p.books.count(), 1)

[Sarah Boyce]

Thank you for the report and test!

[Johanan Oppong Amoateng]

Replying to JaeHyuckSa:

Are you still working on this?

[JaeHyuckSa]

Hello! I forgot that Johanan had a ticket. At the moment, I don’t have time to work on this, so please feel free to take it on if you’re able to.

[Vasilis Vagenas]

Hello.
The problem can still be reproduced using the test method get_or_create.tests.GetOrCreateTests.test_get_or_create_on_related_queryset, mentioned above by Nick Pope.

• This test method was added after checking out :
  • ed79c5959add54b6e8ea589ec601e0d2e801517e (branch: main)
  • 29e31fbe87d56d0b2ee824c77113e56ed4a44f64 (branch: stable/6.0.x)
  • fb61c8a6e902abc885048a1a78592a4bd4329f87 (branch: stable/5.2.x)
• The IntegrityError is not thrown (and the test succeeds) based on either of the aforementioned Git commits, if we change the get_or_create call to:
  • book, created = p.books.all().get_or_create(name="The Book of Ed & Fred", publisher=p)
• This agrees with what Nick Pope suggested that ".get_or_create() and .update_or_create() lose context of the related instance"
• the test output for the case of the main branch can be found below:

======================================================================
ERROR: test_get_or_create_on_related_queryset (get_or_create.tests.GetOrCreateTests.test_get_or_create_on_related_queryset)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vasilis/oss_projects/django/django/django/db/models/query.py", line 1029, in get_or_create
    return self.get(**kwargs), False
           ^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/query.py", line 681, in get
    raise self.model.DoesNotExist(
get_or_create.models.Book.DoesNotExist: Book matching query does not exist.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/vasilis/oss_projects/django/django/django/db/backends/utils.py", line 105, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/backends/sqlite3/base.py", line 359, in execute
    return super().execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.IntegrityError: NOT NULL constraint failed: get_or_create_book.publisher_id_column

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/vasilis/oss_projects/django/django/tests/get_or_create/tests.py", line 159, in test_get_or_create_on_related_queryset
    book, created = p.books.all().get_or_create(name="The Book of Ed & Fred")
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/query.py", line 1036, in get_or_create
    return self.create(**params), True
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/query.py", line 711, in create
    obj.save(force_insert=True, using=self.db)
  File "/home/vasilis/oss_projects/django/django/django/db/models/base.py", line 896, in save
    self.save_base(
  File "/home/vasilis/oss_projects/django/django/django/db/models/base.py", line 988, in save_base
    updated = self._save_table(
              ^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/base.py", line 1191, in _save_table
    results = self._do_insert(
              ^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/base.py", line 1243, in _do_insert
    return manager._insert(
           ^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/query.py", line 2108, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/models/sql/compiler.py", line 1935, in execute_sql
    cursor.execute(sql, params)
  File "/home/vasilis/oss_projects/django/django/django/db/backends/utils.py", line 79, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/backends/utils.py", line 92, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/backends/utils.py", line 100, in _execute
    with self.db.wrap_database_errors:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/utils.py", line 94, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/vasilis/oss_projects/django/django/django/db/backends/utils.py", line 105, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vasilis/oss_projects/django/django/django/db/backends/sqlite3/base.py", line 359, in execute
    return super().execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.IntegrityError: NOT NULL constraint failed: get_or_create_book.publisher_id_column

----------------------------------------------------------------------

[Vasilis Vagenas]

Some more notes on the issue:

• As also specified in the pull request, the create method of the queryset is the root of the issue; e.g. the following test shows the issue by throwing an IntegrityError:

      def test_create_on_related_queryset(self):
          p = Publisher.objects.create(name="Acme Publishing")
          p.books.all().create(name="The Book of Ed & Fred")
          self.assertEqual(p.books.count(), 1)
  

• A question raised is what the behavior should be in the case of queryset operations which create ambiguity for the implicit related object (e.g. for publisher in this case). A typical example is the union of querysets, when the querysets have different implicit related objects. For example, should the test below continue throwing an IntegrityError, i.e. leaving the publisher equal to None?

      def test_create_on_union_of_querysets_when_publisher_not_set(self):
          p1 = Publisher.objects.create(name="Acme Publishing")
          p2 = Publisher.objects.create(name="Packt Publishing")
          books_union = p1.books.all().union(p2.books.all())
          books_union.create(name="The Book of Ed & Fred")
          self.assertEqual(Book.objects.count(), 1)
  

• Implementation notes:
  • in the test above, books_union._known_related_objects has just 1 publisher, so this attribute currently does not support the detection of the ambiguous case.
  • any future fix should not override a foreign key relation specified explicitly, e.g. the following should succeed:

        def test_create_on_union_of_querysets_when_publisher_set(self):
            p1 = Publisher.objects.create(name="Acme Publishing")
            p2 = Publisher.objects.create(name="Packt Publishing")
            books_union = p1.books.all().union(p2.books.all())
            books_union.create(name="The Book of Ed & Fred", publisher=p2)
            self.assertEqual(p2.books.count(), 1)
    

  • regarding all 3 test methods above and the test_get_or_create_on_related_queryset mentioned above, a draft implementation that makes them succeed could be to add to the beginning of django.db.models.query.QuerySet.create the following snippet:

            for field in self._known_related_objects:
                field_dict = self._known_related_objects[field]
                if len(field_dict) == 1:
                    field_id_name = f'{field.name}_id'
                    field_explicitly_specified = field.name in kwargs or field_id_name in kwargs
                    if not field_explicitly_specified:
                        model_instance = list(field_dict.values())[0]
                        kwargs[field.name] = model_instance