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 = , sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = () ignored_wrapper_args = (False, {'connection': , 'cursor': }) 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 = , 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 = , 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 = , sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = (), many = False executor = > 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 = , sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = () ignored_wrapper_args = (False, {'connection': , 'cursor': }) 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 = , exc_type = 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 = 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 = , sql = 'SELECT "test_type"."id" FROM "test_type" ORDER BY "test_type"."id" ASC', params = () ignored_wrapper_args = (False, {'connection': , 'cursor': }) 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 = >, django_db_blocker = @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 = , 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.