[Django] #26789: ORM produces query with NULL instead of empty geometry
Django
-------------------------------+--------------------
Reporter: sir-sigurd | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
{{{#!python
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
{{{#!python
/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
{{{#!python
from django.db import connection
connection.queries[-1]['sql']
}}}
is
{{{#!python
u'INSERT INTO "test_app_city" ("point") VALUES (NULL) RETURNING
"test_app_city"."id"'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26789>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Uncategorized | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sir-sigurd):
* status: new => assigned
* needs_better_patch: => 0
* component: Uncategorized => GIS
* needs_tests: => 0
* owner: nobody => sir-sigurd
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:1>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:2>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jtiai):
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:3>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rcoup):
* cc: robert.coup@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:4>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by rcoup):
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:5>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rcoup):
* has_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:6>
Django
Reporter: sir-sigurd | Owner: sir-sigurd
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:7>
Django
Reporter: Sergey Fedoseev | Owner: Sergey
| Fedoseev
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/7428 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:8>
Django
Reporter: Sergey Fedoseev | Owner: Sergey
| Fedoseev
Type: Bug | Status: assigned
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:9>
Django
Reporter: Sergey Fedoseev | Owner: Sergey
| Fedoseev
Component: GIS | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"183f5015408ffe0d3a22203ddaf7a3a56da025fd" 183f501]:
{{{
#!CommitTicketReference repository=""
revision="183f5015408ffe0d3a22203ddaf7a3a56da025fd"
Fixed #26789 -- Fixed handling of empty geometries in
BaseSpatialField.get_db_prep_save().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:11>
Django
Reporter: Sergey Fedoseev | Owner: Sergey
| Fedoseev
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b90d72facf1e4294df1c2e6b51b26f6879bf2992" b90d72fa]:
{{{
#!CommitTicketReference repository=""
revision="b90d72facf1e4294df1c2e6b51b26f6879bf2992"
Refs #26789 -- Fixed output of WKBWriter for empty points and polygons.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26789#comment:10>