CompositeType.Operation will create current type definition, not historical definition

[mx-moth]

The current implementation of CompositeType.Operation will create the type as it is when the migration is run, not as the type was when the migration was created. This can lead to problems when changing the type later, and making a migration for it. Consider the following scenario:

1. A type is created with two fields.
2. A migration is made that creates this type
3. The migrations are run
4. A new field is added to the type
5. A new migration is created to add this new field to the type
6. The migrations are run

This will work. However, for someone making a new database from scratch, this will fail. The type will be created with all three fields in the first migration, and the second migration will fail to add the duplicate field.

The creation migration should save the state of the type at the time the migration is created, rather than the time the migration is run.

[achidlow]

I've had a look at the django migration code, it seems pretty inflexible. The only way I can think to do this right now without abusing undocumented API's is by extending the makemigrations command.

Thoughts?

(Bonus: this would also prevent the need for manually adding the operation to migrations.)

[mx-moth]

That is the same conclusion I came to. That does come with its own set of problems though, as you would have to effectively maintain a fork of the Django migration code, possibly against multiple versions of Django, for this to work.

The best approach would be to make Django migrations more flexible, allowing other apps to hook in to the autodetection step, and allowing things other than models to participate in the migration workflow. This is a big task though.

[danni]

I think the best outcome would be to add a new command that output operations just for types (even requiring people to copy them into their migrations manually). And then remove the magic-generated operations for a more conventional CreateType, AlterType, DropType set of operations.

[achidlow]

After taking a crack at this, I can see this could be done, but it's a bit too much work for me right now. I think you'd necessarily have to end up re-implementing a lot of existing django migrations code.

[jleclanche]

I'm experimenting with a different approach that may or may not yield results: https://github.com/financica/django-postgres-composite-types/tree/dev/compat-with-model

My theory is that a composite type is similar to a model; so, it should be possible to make Django think that it is a Model, and hack into Django so that it generates the types and selects them correctly.

Success 1: By subclassing CreateModel migration operation into a CreateType operation, I've been able to create composite types without issues. However, the migration generated by makemigrations needs hand editing as there is no way to tell it to use that particular operation instead of the CreateModel operation, which is hardcoded. This is easily fixed in Django.

Success 2: I've renamed db_type to db_table, making the composite type compatible with Model. This has so far removed quite a bit of hacky code.

Fail 1: Models absolutely require a primary key in Django. I have however worked around this by generating a dummy __id column, which I ignore during field generation. I believe this will be a huge blocker for this approach if the code is upstreamed into Django itself.

Current blocker: Tests are failing due to the following:

================================================================================================================== test session starts ===================================================================================================================
platform linux -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0
django: settings: tests.settings (from env)
rootdir: /home/adys/src/financica/django-postgres-composite-types
plugins: django-4.5.2
collected 29 items

tests/test_field.py E

========================================================================================================================= ERRORS =========================================================================================================================
_____________________________________________________________________________________________________ ERROR at setup of FieldTests.test_adapted_sql ______________________________________________________________________________________________________
self = <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>, sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = ()
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f5d3a587340>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.WrongObjectType: "test_type" is a composite type
E               LINE 1: ...CURSOR WITH HOLD FOR SELECT "test_type"."id" FROM "test_type...
E                                                                            ^

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/utils.py:84: WrongObjectType

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

self = <django.db.models.sql.compiler.SQLCompiler object at 0x7f5d3a3a0af0>, result_type = 'multi', chunked_fetch = True, chunk_size = 2000

    def execute_sql(self, result_type=MULTI, chunked_fetch=False, chunk_size=GET_ITERATOR_CHUNK_SIZE):
        """
        Run the query against the database and return the result(s). The
        return value is a single data item if result_type is SINGLE, or an
        iterator over the results if the result_type is MULTI.

        result_type is either MULTI (use fetchmany() to retrieve all rows),
        SINGLE (only retrieve a single row), or None. In this last case, the
        cursor is returned if any query is executed, since it's used by
        subclasses such as InsertQuery). It's possible, however, that no query
        is needed, as the filters describe an empty set. In that case, None is
        returned, to avoid any unnecessary database interaction.
        """
        result_type = result_type or NO_RESULTS
        try:
            sql, params = self.as_sql()
            if not sql:
                raise EmptyResultSet
        except EmptyResultSet:
            if result_type == MULTI:
                return iter([])
            else:
                return
        if chunked_fetch:
            cursor = self.connection.chunked_cursor()
        else:
            cursor = self.connection.cursor()
        try:
>           cursor.execute(sql, params)

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/models/sql/compiler.py:1175:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>, sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = ()

    def execute(self, sql, params=None):
>       return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/utils.py:66:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>, sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = (), many = False
executor = <bound method CursorWrapper._execute of <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>>

    def _execute_with_wrappers(self, sql, params, many, executor):
        context = {'connection': self.db, 'cursor': self}
        for wrapper in reversed(self.db.execute_wrappers):
            executor = functools.partial(wrapper, executor)
>       return executor(sql, params, many, context)

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/utils.py:75:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>, sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = ()
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f5d3a587340>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
>       with self.db.wrap_database_errors:

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/utils.py:79:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.utils.DatabaseErrorWrapper object at 0x7f5d3a3a1390>, exc_type = <class 'psycopg2.errors.WrongObjectType'>
exc_value = WrongObjectType('"test_type" is a composite type\nLINE 1: ...CURSOR WITH HOLD FOR SELECT "test_type"."id" FROM "test_type...\n                                                             ^\n')
traceback = <traceback object at 0x7f5d3a3d33c0>

    def __exit__(self, exc_type, exc_value, traceback):
        if exc_type is None:
            return
        for dj_exc_type in (
                DataError,
                OperationalError,
                IntegrityError,
                InternalError,
                ProgrammingError,
                NotSupportedError,
                DatabaseError,
                InterfaceError,
                Error,
        ):
            db_exc_type = getattr(self.wrapper.Database, dj_exc_type.__name__)
            if issubclass(exc_type, db_exc_type):
                dj_exc_value = dj_exc_type(*exc_value.args)
                # Only set the 'errors_occurred' flag for errors that may make
                # the connection unusable.
                if dj_exc_type not in (DataError, IntegrityError):
                    self.wrapper.errors_occurred = True
>               raise dj_exc_value.with_traceback(traceback) from exc_value

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/utils.py:90:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>, sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = ()
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f5d3a587340>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f5d3a54ee30>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               django.db.utils.ProgrammingError: "test_type" is a composite type
E               LINE 1: ...CURSOR WITH HOLD FOR SELECT "test_type"."id" FROM "test_type...
E                                                                            ^

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/utils.py:84: ProgrammingError

During handling of the above exception, another exception occurred:

request = <SubRequest '_django_setup_unittest' for <TestCaseFunction test_adapted_sql>>, django_db_blocker = <pytest_django.plugin._DatabaseBlocker object at 0x7f5d3c7d8310>

    @pytest.fixture(autouse=True, scope="class")
    def _django_setup_unittest(
        request,
        django_db_blocker: "_DatabaseBlocker",
    ) -> Generator[None, None, None]:
        """Setup a django unittest, internal to pytest-django."""
        if not django_settings_is_configured() or not is_django_unittest(request):
            yield
            return

        # Fix/patch pytest.
        # Before pytest 5.4: https://github.com/pytest-dev/pytest/issues/5991
        # After pytest 5.4: https://github.com/pytest-dev/pytest-django/issues/824
        from _pytest.unittest import TestCaseFunction
        original_runtest = TestCaseFunction.runtest

        def non_debugging_runtest(self) -> None:
            self._testcase(result=self)

        try:
            TestCaseFunction.runtest = non_debugging_runtest  # type: ignore[assignment]

>           request.getfixturevalue("django_db_setup")

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/pytest_django/plugin.py:490:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/pytest_django/fixtures.py:122: in django_db_setup
    db_cfg = setup_databases(
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/test/utils.py:179: in setup_databases
    connection.creation.create_test_db(
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/base/creation.py:90: in create_test_db
    self.connection._test_serialized_contents = self.serialize_db_to_string()
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/base/creation.py:136: in serialize_db_to_string
    serializers.serialize("json", get_objects(), indent=None, stream=out)
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/core/serializers/__init__.py:129: in serialize
    s.serialize(queryset, **options)
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/core/serializers/base.py:90: in serialize
    for count, obj in enumerate(queryset, start=1):
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/backends/base/creation.py:133: in get_objects
    yield from queryset.iterator()
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/models/query.py:353: in _iterator
    yield from self._iterable_class(self, chunked_fetch=use_chunked_fetch, chunk_size=chunk_size)
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/models/query.py:51: in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.models.sql.compiler.SQLCompiler object at 0x7f5d3a3a0af0>, result_type = 'multi', chunked_fetch = True, chunk_size = 2000

    def execute_sql(self, result_type=MULTI, chunked_fetch=False, chunk_size=GET_ITERATOR_CHUNK_SIZE):
        """
        Run the query against the database and return the result(s). The
        return value is a single data item if result_type is SINGLE, or an
        iterator over the results if the result_type is MULTI.

        result_type is either MULTI (use fetchmany() to retrieve all rows),
        SINGLE (only retrieve a single row), or None. In this last case, the
        cursor is returned if any query is executed, since it's used by
        subclasses such as InsertQuery). It's possible, however, that no query
        is needed, as the filters describe an empty set. In that case, None is
        returned, to avoid any unnecessary database interaction.
        """
        result_type = result_type or NO_RESULTS
        try:
            sql, params = self.as_sql()
            if not sql:
                raise EmptyResultSet
        except EmptyResultSet:
            if result_type == MULTI:
                return iter([])
            else:
                return
        if chunked_fetch:
            cursor = self.connection.chunked_cursor()
        else:
            cursor = self.connection.cursor()
        try:
            cursor.execute(sql, params)
        except Exception:
            # Might fail for server-side cursors (e.g. connection closed)
>           cursor.close()
E           psycopg2.errors.InvalidCursorName: cursor "_django_curs_140038440982336_sync_1" does not exist

../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/django/db/models/sql/compiler.py:1178: InvalidCursorName
----------------------------------------------------------------------------------------------------------------- Captured stderr setup ------------------------------------------------------------------------------------------------------------------
Got an error creating the test database: database "test_postgres" already exists

==================================================================================================================== warnings summary ====================================================================================================================
../../../.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/nose/importer.py:12
  /home/adys/.cache/pypoetry/virtualenvs/django-postgres-composite-types-jpPfru7l-py3.10/lib/python3.10/site-packages/nose/importer.py:12: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
    from imp import find_module, load_module, acquire_lock, release_lock

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================================================================ short test summary info =================================================================================================================
ERROR tests/test_field.py::FieldTests::test_adapted_sql - psycopg2.errors.InvalidCursorName: cursor "_django_curs_140038440982336_sync_1" does not exist
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================================================================== 1 warning, 1 error in 0.86s ===============================================================================================================

I'm not quite sure how to fix this yet, will keep investigating later.

[danni]

test_fields is failing for me also in the main branch.

[jleclanche]

Looking at all these test failures now.