Models with a BinaryField fail to deepcopy in setUpTestData() on PostgreSQL.

[Adam Zimmerman]

While using Postgres, models with a BinaryField fail to deepcopy, due to Django returning these field values as memoryview objects. This causes issues when creating instances in setUpTestData() during tests.

The specific errors seem to be fairly inconsistent and hard to reproduce. But often the exception complains about a missing _state field, or that _state has no attribute db.

I've currently worked around this by implementing a mixin for models where the __getstate__() method converts any memoryviews to bytes. But I don't know if that's the correct solution for Django generally.

[Adam Zimmerman]

For anyone who finds this ticket and wants to work around it before it's fixed in Django, here's the mixin I'm currently using:

class ContainsBinaryField:
    """A mixin for model classes that contain BinaryField fields

    Django 3.2 has started using deepcopy() to isolate changes to data created in
    setUpTestData(). This seems to cause issues with models that have a BinaryField in
    them. This implements the __getstate__() method in a way that converts any
    memoryview data to bytes, which seems to fix the issue."""

    def __getstate__(self):
        state = super().__getstate__()
        for key, value in state.items():
            if isinstance(value, memoryview):
                state[key] = bytes(value)
        return state

[Mariusz Felisiak]

Thanks for the report! Regression in 3cf80d3fcf7446afdde16a2be515c423f720e54d. I was able to reproduce this issue on PostgreSQL with the following test:

class MyTests(TestCase):
    @classmethod
    def setUpTestData(cls):
        MyModel.objects.create(binary=b'y')
        cls.my_binary = MyModel.objects.get()

    def test_1(self):
        self.my_binary.binary = b'z'
        self.my_binary.save()

[Simon Charette]

Feels like the issue should be solved at the model level because while it break setupTestData (3cf80d3fcf7446afdde16a2be515c423f720e54d) it also breaks any form of QuerySet or Model pickling

MyModel.objects.create(binary=b'y')
pickle.dumps(MyModel.objects.all())

This can be reproduced by adding a BinaryField to ​models in the test queryset_pickle apps.

[Simon Charette]

My first instinct would be to have BinaryField.from_db_value use force_bytes to store bytes instead of memoryview in _state or introduce a hook to allow Field subclass to define a hook using during serialization and have Model.__getstate__ call it.

To me that's more of a long standing issue with BinaryField and less of a regression cause by 3cf80d3fcf7446afdde16a2be515c423f720e54d.

[Claude Paroz]

Simon, your diagnostic might be right, but I think it's late in the 4.0 release cycle to make such changes now, also considering changing memoryview to bytes might have serious memory implications not plainly visible in tests.

What about writing a workaround for setuptestData in 4.0, and make a more proper and thorough fix in 4.1?

[Simon Charette]

Given we're pretty close to the 4.0 release date I'm not against including a limited hack for setUpTestData that special cases Model and QuerySet instances.

It does feel weird that we're treating this as a release blocker now though given it has been broken since BinaryField was introduced and setUpTestData ​has been emitting deprecation warnings about it since Django 3.2.

[Mariusz Felisiak]

We have also a ticket about BinaryField inconsistency, see #27813.

What about writing a workaround for setuptestData in 4.0 ...

For me it is a 3.2 release blocker.

[Simon Charette]

considering changing memoryview to bytes might have serious memory implications not plainly visible in tests.

Fixing #27813 would be somewhat of a breaking change on PostgreSQL as a different type would be returned but I doubt it would have memory implications in the sense of elevated memory usage.

In the end memoryview is a pointer to a sequence of bytes but these bytes have to live somewhere when returned from the database backend and made available to the client. Returning them proxied by a memoryview instance or not should not make a significant difference in memory usage beyond the size of the memoryview instance itself.

---

Mariusz, how do you want to proceed here? I have to admit I'm not sure how to navigate the current state of things.

Should we submit a patch meant to be backported to 3.2 and 4.0 in setUpTestData that solely deals with this particular edge case of models and queryset with a BinaryField on Postgres to circumvent #27813? What if we get other reports of users who ignored the deprecation warning for other non-pickleable objects assigned during setUpTestData?

[Mariusz Felisiak]

Should we submit a patch meant to be backported to 3.2 and 4.0 in setUpTestData that solely deals with this particular edge case of models and queryset with a BinaryField on Postgres to circumvent #27813?

Yes, it should be feasible by juggling with __getstate__()/__reduce__() hooks 🤔 I think we should treat this as a release blocker because BinaryField is built-in into Django and 3cf80d3fcf7446afdde16a2be515c423f720e54d broke previously working tests.

What if we get other reports of users who ignored the deprecation warning for other non-pickleable objects assigned during setUpTestData?

That would not be a release blocker, IMO, unless any other built-in is non-pickleable.

[Mariusz Felisiak]

​PR

[GitHub <noreply@…>]

In 2c7846d:

Fixed #33333 -- Fixed setUpTestData() crash with models.BinaryField on PostgreSQL.

This makes models.BinaryField pickleable on PostgreSQL.

Regression in 3cf80d3fcf7446afdde16a2be515c423f720e54d.

Thanks Adam Zimmerman for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 2c20883:

[4.0.x] Fixed #33333 -- Fixed setUpTestData() crash with models.BinaryField on PostgreSQL.

This makes models.BinaryField pickleable on PostgreSQL.

Regression in 3cf80d3fcf7446afdde16a2be515c423f720e54d.

Thanks Adam Zimmerman for the report.

Backport of 2c7846d992ca512d36a73f518205015c88ed088c from main.

[Mariusz Felisiak <felisiak.mariusz@…>]

In cb724ef6:

[3.2.x] Fixed #33333 -- Fixed setUpTestData() crash with models.BinaryField on PostgreSQL.

This makes models.BinaryField pickleable on PostgreSQL.

Regression in 3cf80d3fcf7446afdde16a2be515c423f720e54d.

Thanks Adam Zimmerman for the report.

Backport of 2c7846d992ca512d36a73f518205015c88ed088c from main.

[GitHub <noreply@…>]

In d3a64be:

Refs #33333 -- Fixed PickleabilityTestCase.test_annotation_with_callable_default() crash on Oracle.

Grouping by LOBs is not allowed on Oracle. This moves a binary field to
a separate model.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7bde53a:

[4.0.x] Refs #33333 -- Fixed PickleabilityTestCase.test_annotation_with_callable_default() crash on Oracle.

Grouping by LOBs is not allowed on Oracle. This moves a binary field to
a separate model.
Backport of d3a64bea51676fcf8a0ae593cf7b103939e12c87 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6014b812:

[3.2.x] Refs #33333 -- Fixed PickleabilityTestCase.test_annotation_with_callable_default() crash on Oracle.

Grouping by LOBs is not allowed on Oracle. This moves a binary field to
a separate model.
Backport of d3a64bea51676fcf8a0ae593cf7b103939e12c87 from main