Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#35238 closed Bug (fixed)

Missing chunk_size kwarg throws exception when creating test DB if model has base-manager with prefetches

Reported by: alexcleduc Owned by: alexcleduc
Component: Core (Serialization) Version: 5.0
Severity: Release blocker Keywords:
Cc: alexcleduc Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am trying to upgrade from 4.x to 5.0 and am running into this issue when creating my test database.

I'm using pytest, but I'd be very surprised if this wasn't also an issue with unit-test. That said, I do find it a little surprising no one has noticed this. Surely there would have been other projects out with models whose base-managers have prefetches? So I'm not entirely confident this is a bug in django. Maybe pytest should be initializing test DBs differently, I haven't done that research.

See django/db/backends/base/creation.py for the offending code:

class BaseDatabaseCreation:
   # ...
    def serialize_db_to_string(self):
        # ... 
        # Iteratively return every object for all models to serialize.
        def get_objects():
            for app_config in apps.get_app_configs():
                if (...):
                    for model in app_config.get_models():
                        if (...):
                            queryset = model._base_manager.using(
                                self.connection.alias,
                            ).order_by(model._meta.pk.name)
                            yield from queryset.iterator() # HERE THROWS EXCEPTION

Here is the call-stack in my project, you'll see that although it starts with pytest, there's a chain of 6 first-party django calls that lead to the exception:

  ...
  
  File "/.../site-packages/pytest_django/fixtures.py", line 139, in django_db_setup
    db_cfg = setup_databases(
  File ".../site-packages/django/test/utils.py", line 203, in setup_databases
    connection.creation.create_test_db(
  File ".../site-packages/django/db/backends/base/creation.py", line 94, in create_test_db
    self.connection._test_serialized_contents = self.serialize_db_to_string()
  File ".../site-packages/django/db/backends/base/creation.py", line 142, in serialize_db_to_string
    serializers.serialize("json", get_objects(), indent=None, stream=out)
  File ".../site-packages/django/core/serializers/__init__.py", line 134, in serialize
    s.serialize(queryset, **options)
  File ".../site-packages/django/core/serializers/base.py", line 108, in serialize
    for count, obj in enumerate(queryset, start=1):
  File ".../site-packages/django/db/backends/base/creation.py", line 138, in get_objects
    yield from queryset.iterator()

Proposed solution
I am putting up a PR that will conditionally use the chunk_size kwarg when calling queryset.iterator() in django/db/backends/base/creation.py. An alternative is to set a fallback chunk_size in iterator() itself, if prefetches are found, but I am guessing there's a good reason that wasn't done in the first place. I am defaulting to the same as aiterator, which is 2000

I am not confident in this solution, there seem to be quite a bit of first-party calls to queryset.iterator that don't include a chunk-size kwarg, however this fix makes my project's relatively large test-suite pass, and I have prefetches on many models' base-managers including the user model.

Change History (7)

comment:1 by Simon Charette, 3 months ago

Triage Stage: UnreviewedAccepted

comment:2 by Mariusz Felisiak, 3 months ago

Has patch: unset

comment:3 by Mariusz Felisiak, 3 months ago

Has patch: set
Needs tests: set
Severity: NormalRelease blocker

comment:4 by Mariusz Felisiak, 3 months ago

Needs documentation: set
Owner: changed from nobody to alexcleduc
Status: newassigned

comment:5 by Mariusz Felisiak, 3 months ago

Needs documentation: unset
Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 3 months ago

Resolution: fixed
Status: assignedclosed

In a084c5d3:

Fixed #35238 -- Fixed database serialization crash when base managers use prefetch_related().

Regression in 139135627650ed6aaaf4c755b82c3bd43f2b8f51
following deprecation in eedbf930287cb72e9afab1f7208c24b1146b0c4ec.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 3 months ago

In 69e5b13:

[5.0.x] Fixed #35238 -- Fixed database serialization crash when base managers use prefetch_related().

Regression in 139135627650ed6aaaf4c755b82c3bd43f2b8f51
following deprecation in eedbf930287cb72e9afab1f7208c24b1146b0c4ec.

Backport of a084c5d35a6d00abd261338a374a4424764b4aee from main

Note: See TracTickets for help on using tickets.
Back to Top