CheckConstraint crashes with GIS lookup and PostGIS, MySQL, and Oracle backends.

[Daniel Swain]

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()

[Mariusz Felisiak]

Thanks for the report.

Reproduced at 022d29c934107c515dd6d3181945146a2077bdf0.

[Arsalan Ghassemi]

Hi,

I'm working on a fix, the topic branch : ​https://github.com/ArsaCode/django/tree/ticket_33047

[Arsalan Ghassemi]

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 :

Polygon Assertions

The boundary of a Polygon consists of a set of LinearRing objects (that is, LineString objects that are both simple and closed) that make up its exterior and interior boundaries.

A Polygon has no rings that cross. The rings in the boundary of a Polygon may intersect at a Point, but only as a tangent.

A Polygon has no lines, spikes, or punctures.

A Polygon has an interior that is a connected point set.

A Polygon may have holes. The exterior of a Polygon with holes is not connected. Each hole defines a connected component of the exterior.

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 :)

[Claude Paroz]

My ​PR based on Arsalan version.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 64c3f049:

Fixed #33047 -- Fixed CheckConstraint crash with GIS lookups on PostGIS and MySQL GIS backends.

Thanks Daniel Swain for the report and Arsalan Ghassemi for the initial
patch.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>