Opened 4 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#36129 closed Bug (invalid)

Geodjango migrations ignore change of dimension in MultiPointField

Reported by: Paweł Kotiuk Owned by:
Component: GIS Version: dev
Severity: Normal Keywords: migrations
Cc: Claude Paroz Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I think I found a bug in GeoDjango migration mechanism.

Reproduction

  1. Create a simple model 
class MultiPointTester(models.Model):
  multipoint = models.MultiPointField(
      dim=2,
      blank=True,
      null=True,
      default="SRID=4326;MULTIPOINT EMPTY",
  )

and test for it

class MultipointTest(TransactionTestCase):
    def test_multipoint_creation(self):
        instance = models.MultiPointTester()
        instance.save()
        self.assertEqual(instance.multipoint.ewkt, "SRID=4326;MULTIPOINT EMPTY")
  1. Generate migration for it ./manage.py makemigrations
  2. Run test - it succeeds ✅
  3. Change dim from 2 to 3 and create migration It contains migration like this (it does not mention dim change): 
      operations = [
      migrations.AlterField(
          model_name='multipointtester',
          name='multipoint',
          field=django.contrib.gis.db.models.fields.MultiPointField(
                blank=True, 
                default='SRID=4326;MULTIPOINT EMPTY', 
                null=True, srid=4326),
      ),
  ]
  1. Run test again - it fails ❌ with error: E ValueError: Cannot alter field test.MultiPointTester.multipoint into test.MultiPointTester.multipoint - they do not properly define db_type (are you using a badly-written custom field?)
  2. Remove all existing migration files and again generate migration ./manage.py makemigrations
  3. Run test again - - it succeeds ✅

Is is a bug, or do I make some kind of mistake in the code?

Change History (5)

comment:1 by Paweł Kotiuk, 4 weeks ago

This bug may linked with

#28809 and #29058

comment:2 by Natalia Bidart, 4 weeks ago

Cc: Claude Paroz added
Keywords: migrations added
Resolution: → invalid
Status: new → closed
Type: Uncategorized → Bug
Version: 5.1 → dev

Hello Pawel, thank you for this report. I agree this is similar to the tickets you linked and also to #23902. Can you specify what backend are you using?

Based on the previous tickets, and following the comments from Claude, it seems quite challenging to generate a migration that makes sense for the backend, in that how the added dimension would work with respect to the existing values in the DB? For your reproducer specifically, my understanding is that your last step works because the test DB is created with a single and "clean" migration where the dim=3 since the start.

Having said that, I have tried this in a test project and using the admin to create a point when dim=2. Then I changed to dim=3 and the point was migrated in the DB as follows (this is PostGIS): SRID=4326;MULTIPOINT Z ((-0.0051498413085937 0.0168228146996821 0)). But I can't add new points because of the chosen default, which does not have the third dimension:

Exception Type: DataError at /admin/geodjangoapp/multipointtester/add/
Exception Value: Column has Z dimension but geometry does not

So, I think that this specific report is about the default being incorrect for a 3-dimension multipoint (which would consitute an user error), and not so much about migration generation failure, but I'm not a GIS expert. I was going to propose for you to reach our in the forum, but I see you have already ​done so. Because of my local testing, I'll be closing this ticket as invalid following the ​ticket triaging process, but feel free to reopen if you find more specific information detailing that Django is at fault in this specific case, specifically once you remove the potentially incorrect default in place.

Thanks again!

comment:3 by Paweł Kotiuk, 4 weeks ago

Hello Natalia, thank you for quick response.
Originally I discovered this bug, when I wanted to decrease number of dimensions, and I was surprised that every single unit test in my test suite failed. And generated migration (even for database generated from scratch) always fails with: ValueError: Cannot alter field...

Can you specify what backend are you using?

My database backend is django.contrib.gis.db.backends.spatialite

in that how the added dimension would work with respect to the existing values in the DB?

For now migration breaks entire database, even when it is recreated from scratch only for tests.
I think it should work in the same way as migration for providing new required value for model. So we should at least get a prompt asking about default value we want to use.

it seems quite challenging to generate a migration that makes sense for the backend

In case of migration from 2 to 3 dimensions yes, but in case of migrating from 3 dimensions to 2 it should be rather simple. Just delete Z axis.
I think , that at least this single case should be somehow tackled (and in other cases some warning should be shown).

I think this issue should be reopened, because it causes unexpected problems with django project by generating broken migrations and it should be somehow tackled. Either by providing reliable migration mechanism or at least by showing warning during migration or generating migration files.

comment:4 by Paweł Kotiuk, 4 weeks ago

find more specific information detailing that Django is at fault in this specific case, specifically once you remove the potentially incorrect default in place.

This problem also exists when there is no default.

You can try to reproduce with simplified model without default (and simpler tests)

model 

class MultiPointTester(models.Model):
    multipoint = models.MultiPointField(
        dim=3,
        blank=True,
        null=True,
    )

test 

class MultipointTest(TransactionTestCase):
    def test_multipoint_creation(self):
        instance = models.MultiPointTester()
        instance.save()

    def test_always_working(self):
        self.assertEqual(1, 1)

Migration from 3 to 2 still breaks every test (even test_always_working)

in reply to:  4 comment:5 by Natalia Bidart, 4 weeks ago

Replying to Paweł Kotiuk:

Migration from 3 to 2 still breaks every test (even test_always_working)

Thank you Paweł for the extra information though sadly I'm not being able to reproduce (though I'm using PostGIS). Can you please try these two things:

  1. Can you use TestCase instead of TransactionTestCase.
  2. Can you try to reproduce using PostGIS?

I tried all sort of migrations from dim 3 to 2, then from 2 to 3, and adding points in the admin always work. Also the test as shown (but using TestCase) always passes for me as well.

Thank you!

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