ORM produces query with NULL instead of empty geometry

[Sergey Fedoseev]

from test_app.models import City
from django.contrib.gis.geos.geometry import GEOSGeometry

City.objects.create(point=GEOSGeometry('POINT EMPTY', srid=4326))

raises error

/home/sergey/dev/django/django/db/models/manager.pyc in manager_method(self, *args, **kwargs)
     83         def create_method(name, method):
     84             def manager_method(self, *args, **kwargs):
---> 85                 return getattr(self.get_queryset(), name)(*args, **kwargs)
     86             manager_method.__name__ = method.__name__
     87             manager_method.__doc__ = method.__doc__

/home/sergey/dev/django/django/db/models/query.pyc in create(self, **kwargs)
    397         obj = self.model(**kwargs)
    398         self._for_write = True
--> 399         obj.save(force_insert=True, using=self.db)
    400         return obj
    401 

/home/sergey/dev/django/django/db/models/base.pyc in save(self, force_insert, force_update, using, update_fields)
    794 
    795         self.save_base(using=using, force_insert=force_insert,
--> 796                        force_update=force_update, update_fields=update_fields)
    797     save.alters_data = True
    798 

/home/sergey/dev/django/django/db/models/base.pyc in save_base(self, raw, force_insert, force_update, using, update_fields)
    822             if not raw:
    823                 self._save_parents(cls, using, update_fields)
--> 824             updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
    825         # Store the database on which the object was saved
    826         self._state.db = using

/home/sergey/dev/django/django/db/models/base.pyc in _save_table(self, raw, cls, force_insert, force_update, using, update_fields)
    906 
    907             update_pk = bool(meta.has_auto_field and not pk_set)
--> 908             result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
    909             if update_pk:
    910                 setattr(self, meta.pk.attname, result)

/home/sergey/dev/django/django/db/models/base.pyc in _do_insert(self, manager, using, fields, update_pk, raw)
    945         """
    946         return manager._insert([self], fields=fields, return_id=update_pk,
--> 947                                using=using, raw=raw)
    948 
    949     def delete(self, using=None, keep_parents=False):

/home/sergey/dev/django/django/db/models/manager.pyc in manager_method(self, *args, **kwargs)
     83         def create_method(name, method):
     84             def manager_method(self, *args, **kwargs):
---> 85                 return getattr(self.get_queryset(), name)(*args, **kwargs)
     86             manager_method.__name__ = method.__name__
     87             manager_method.__doc__ = method.__doc__

/home/sergey/dev/django/django/db/models/query.pyc in _insert(self, objs, fields, return_id, raw, using)
   1042         query = sql.InsertQuery(self.model)
   1043         query.insert_values(fields, objs, raw=raw)
-> 1044         return query.get_compiler(using=using).execute_sql(return_id)
   1045     _insert.alters_data = True
   1046     _insert.queryset_only = False

/home/sergey/dev/django/django/db/models/sql/compiler.pyc in execute_sql(self, return_id)
   1052         with self.connection.cursor() as cursor:
   1053             for sql, params in self.as_sql():
-> 1054                 cursor.execute(sql, params)
   1055             if not (return_id and cursor):
   1056                 return

/home/sergey/dev/django/django/db/backends/utils.pyc in execute(self, sql, params)
     77         start = time()
     78         try:
---> 79             return super(CursorDebugWrapper, self).execute(sql, params)
     80         finally:
     81             stop = time()

/home/sergey/dev/django/django/db/backends/utils.pyc in execute(self, sql, params)
     62                 return self.cursor.execute(sql)
     63             else:
---> 64                 return self.cursor.execute(sql, params)
     65 
     66     def executemany(self, sql, param_list):

/home/sergey/dev/django/django/db/utils.pyc in __exit__(self, exc_type, exc_value, traceback)
     92                 if dj_exc_type not in (DataError, IntegrityError):
     93                     self.wrapper.errors_occurred = True
---> 94                 six.reraise(dj_exc_type, dj_exc_value, traceback)
     95 
     96     def __call__(self, func):

/home/sergey/dev/django/django/db/backends/utils.pyc in execute(self, sql, params)
     62                 return self.cursor.execute(sql)
     63             else:
---> 64                 return self.cursor.execute(sql, params)
     65 
     66     def executemany(self, sql, param_list):

IntegrityError: ОШИБКА:  нулевое значение в столбце "point" нарушает ограничение NOT NULL
DETAIL:  Ошибочная строка содержит (3, null).

The last query

from django.db import connection
connection.queries[-1]['sql']

is

u'INSERT INTO "test_app_city" ("point") VALUES (NULL) RETURNING "test_app_city"."id"'

[Jani Tiainen]

Root cause it that geometry value begin nothing is tested with clause if not value . Which in turn calls __len__ on a geometry, having a following responses in different geometries:

For an empty point number of coordinates is returned. In case of empty 0 (zero) is returned (false).

For a linestring empty tuple is returned () (false)

For an polygon empty tuple of tuple is returned ((),) (true)

For a geometry collection length of 0 (zero) is returned (false)

[Robert Coup]

Simplistic fix could be something like (BaseSpatialField):

    def get_db_prep_save(self, value, connection):
        """
        Prepare the value for saving in the database.
        """
        if value or isinstance(value, Geometry):
            return connection.ops.Adapter(self.get_prep_value(value))
        else:
            return None

A test case I used to track down the issue (gis_tests/geoapp/test_regress.py):

    def test_empty_assignment(self):
        c = Country.objects.get(name='Texas')
        c.mpoly = 'SRID=4326;MULTIPOLYGON EMPTY'
        self.assertIsInstance(c.mpoly, geos.MultiPolygon)
        self.assertTrue(c.mpoly.empty)
        c.save()

        c.refresh_from_db()
        self.assertIsInstance(c.mpoly, geos.MultiPolygon)
        self.assertTrue(c.mpoly.empty)

[Robert Coup]

WIP pull request at ​https://github.com/django/django/pull/6934

Current approach passes empty to the DB and lets it decide whether to accept it.

Issues for discussion here:

• POINT EMPTY has a GEOS error on save for me w/ GEOS3.5.0 on OSX. We could workaround that.
• POLYGON EMPTY has a PostGIS error on save for me w/ Postgis 2.2.2 (Postgres 9.5) on OSX
• Should we delegate to backends to "check" whether a geometry is save-able, or pass it through to the DB?
• Need to test on the other DB backends, @jitai is that possible for you to do Oracle? Might need a @no_oracle on the test.

I considered an alternative approach of "fixing" falsey/truthy evaluation of geometries and use that as a proxy to save. Though IMO an empty geom could quite logically evaluate to False though it should be saved.

[Sergey Fedoseev]

​PR

[Tim Graham <timograham@…>]

In b90d72fa:

Refs #26789 -- Fixed output of WKBWriter for empty points and polygons.

[Tim Graham <timograham@…>]

In 183f501:

Fixed #26789 -- Fixed handling of empty geometries in BaseSpatialField.get_db_prep_save().