danni / django-postgres-composite-types

Postgres composite types support for Django
BSD 3-Clause "New" or "Revised" License
51 stars 13 forks source link

Better compat with Model and Django Migrations #31

Open jleclanche opened 1 year ago

jleclanche commented 1 year ago

This branch is an experiment, and it's currently broken.

I mentioned this in #8:

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.

So this is where I'm at. The two main changes are db_type is now db_table (and this is still required; breaking change), and _meta.fields is now the same structure as for normal fields, which is a list of field instances; their name comes from field.name.

I've had to use a custom manager that always returns empty querysets, as the tests surfaced that some django code (such as the db serialization code) will query all models. Types should always be empty for this.

This doesn't solve #8 because something like AlterType hasn't been implemented, but it makes it a lot easier to tackle.

Also, because I replaced the migration operation with one that doesn't rely on the class itself, I have had to drop support for the composite_type_created signal. IMO this is not a useful signal anyway - any push back?

Currently, I'm fighting with the following test error:

>       self.assertIn(expected_deconstruction, text)
E       AssertionError: 'base_field=tests.models.Card.Field()' not found in 'django.contrib.postgres.fields.ArrayField(base_field=postgres_composite_types.composite_type.Card.Field(), size=None)'

I'm still investigating what's happening here.

jleclanche commented 1 year ago

@danni I've fixed all test errors.

The following issues remain in makemigrations: