Opened 3 years ago
Closed 3 years ago
#33047 closed Bug (fixed)
CheckConstraint crashes with GIS lookup and PostGIS, MySQL, and Oracle backends.
Reported by: | Daniel Swain | Owned by: | Claude Paroz |
---|---|---|---|
Component: | GIS | Version: | dev |
Severity: | Normal | Keywords: | constraint, postgres, postgis, geos, attributeerror, migration, schema_editor, getquoted, quote_value |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Trying to apply a migration adding a CheckConstraint
with a spatial lookup (within) fails with: AttributeError: 'str' object has no attribute 'decode'.
when creating the SQL statement.
Traceback:
File "/usr/local/lib/python3.9/site-packages/django/db/migrations/operations/models.py", line 858, in database_forwards schema_editor.add_constraint(model, self.constraint) File "/usr/local/lib/python3.9/site-packages/django/db/backends/base/schema.py", line 379, in add_constraint sql = constraint.create_sql(model, self) File "/usr/local/lib/python3.9/site-packages/django/db/models/constraints.py", line 54, in create_sql check = self._get_check_sql(model, schema_editor) File "/usr/local/lib/python3.9/site-packages/django/db/models/constraints.py", line 47, in _get_check_sql return sql % tuple(schema_editor.quote_value(p) for p in params) File "/usr/local/lib/python3.9/site-packages/django/db/models/constraints.py", line 47, in <genexpr> return sql % tuple(schema_editor.quote_value(p) for p in params) File "/usr/local/lib/python3.9/site-packages/django/db/backends/postgresql/schema.py", line 45, in quote_value return adapted.getquoted().decode()
Investigation
Looking at the DatabaseSchemaEditor
class in the postgresql
backend, it looks like quote_value
expects adapted.getquoted()
to return a bytestring
:
def quote_value(self, value): ... # getquoted() returns a quoted bytestring of the adapted value. return adapted.getquoted().decode()
The adaper used in this migration, PostGISAdapter
returns a str
from getquoted
, not a bytestring
:
class PostGISAdapter: def getquoted(self): """ Return a properly quoted string for use in PostgreSQL/PostGIS. """ if self.is_geometry: # Psycopg will figure out whether to use E'\\000' or '\000'. return '%s(%s)' % ( 'ST_GeogFromWKB' if self.geography else 'ST_GeomFromEWKB', self._adapter.getquoted().decode() ) else: # For rasters, add explicit type cast to WKB string. return "'%s'::raster" % self.ewkb
Example model.
We have a model definition similar to:
from django.contrib.gis.db import models class Example(models.Model): location = models.models.PointField(null=True, blank=True)
We are trying to add a CheckConstraint
that checks that the location is within a bounding box: (x0, y0, x1, y1)
(The rest of these examples will use a bounding box for Australia).
from django.contrib.gis.geos import Polygon class Example(models.Model): location = models.models.PointField(null=True, blank=True) class Meta: constraints = [ models.CheckConstraint( check=models.Q(location__within=Polygon.from_bbox((96.816941, -43.740510, 167.998035, -9.142176)), name="location_in_australia", ), ]
A migration is successfully created:
class Migration(migration.Migration): operations = [ migrations.AddConstraint( model_name='example', constraint=models.CheckConstraint(check=models.Q(('location__within', django.contrib.gis.geos.polygon.Polygon(((96.816941, -43.74051), (96.816941, -9.142176), (167.998035, -9.142176), (167.998035, -43.74051), (96.816941, -43.74051))))), name='location_in_australia'), ), ]
However, trying to run that migration gives the traceback shown at the top.
Potential fix/workaround.
We were able to run the migration by editing the file django/contrib/gis/db/backends/postgis/adapter.py
to make PostGISAdapter.getquoted()
return a bytestring
, but we don't know what effect this will have elsewhere.
Example:
def getquoted(self): if self.is_geometry: # Psycopg will figure out whether to use E'\\000' or '\000'. quoted = '%s(%s)' % ( 'ST_GeogFromWKB' if self.geography else 'ST_GeomFromEWKB', self._adapter.getquoted().decode() ) else: # For rasters, add explicit type cast to WKB string. quoted = "'%s'::raster" % self.ewkb return quoted.encode()
Change History (8)
comment:1 by , 3 years ago
Component: | contrib.postgres → GIS |
---|---|
Owner: | set to |
Summary: | str decode error when applying migration with a CheckConstraint using a GEOS geometry and PostGISAdapter → CheckConstraint crashes with GIS lookup and PostGIS backend. |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Hi,
I'm working on a fix, the topic branch : https://github.com/ArsaCode/django/tree/ticket_33047
comment:3 by , 3 years ago
PR (work in progress) : https://github.com/django/django/pull/15130
With current changes the build fails on mysql_gis :
database=mysql_gis,label=mariadb,python=python3.10
Error :
MySQLdb._exceptions.OperationalError: (1583, "Incorrect parameters in the call to native function 'ST_GeomFromText'")
database=mysql_gis,label=bionic-pr,python=python3.10
database=mysql_gis,label=bionic-pr,python=python3.8
database=mysql_gis,label=bionic-pr,python=python3.9
Error :
MySQLdb._exceptions.OperationalError: (1583, "Incorrect parameters in the call to native function 'st_geometryfromtext'")
It seems like the parameter given to the function "ST_GeomFromText"/"st_geometryfromtext" is invalid. I looked at the Polygon assertions in the MySQL documentation and the values I've put in the regression test migration seems not to be a valid Polygon for MySQL : https://dev.mysql.com/doc/refman/8.0/en/gis-class-polygon.html
I think I'm not testing the patch correctly but I'm not sure how to do a regression test for it (since adding the new test migration was making the test fail before the patch). Any help would be appreciated :)
comment:5 by , 3 years ago
Patch needs improvement: | set |
---|---|
Summary: | CheckConstraint crashes with GIS lookup and PostGIS backend. → CheckConstraint crashes with GIS lookup and PostGIS, MySQL, and Oracle backends. |
comment:6 by , 3 years ago
Owner: | changed from | to
---|
comment:7 by , 3 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Thanks for the report.
Reproduced at 022d29c934107c515dd6d3181945146a2077bdf0.