Opened 4 weeks ago

Last modified 28 hours ago

#37322 assigned Bug

Saving to SpatialField(srid=None) persists NULL instead of raising an error

Reported by: Jacob Walls Owned by: Yassin Bahri
Component: GIS Version: 6.1
Severity: Release blocker Keywords: srid
Cc: Simon Charette 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

At least one user has tried to use spatial fields with a definition of srid=None, see ​forum. I'm pretty certain that's invalid usage.

Django can't create a migration to create such a spatial column with srid=None[0], but you can imagine a user trying this with an unmanaged model.

In Django 6.0, trying to save values to such a field raised DatabaseError, because None compiled to the string "None" and crashed.

After 1a8fd5cf75bf855852f6bc2f75c3da9f7b145669 (#36727, Django 6.1) implemented proper parameterization for srid values in spatial queries, None compiled to NULL, which zeroed out the value (because ST_Transform(..., NULL) returns NULL).

Suggesting we throw an error for this case in Python before letting the query persist NULL.

Test case ​on DryORM:
(You could probably construct a version of this test case using only output_field=GeometryField(srid=None)).)

from django.contrib.gis.db import models
from django.contrib.gis.geos import GEOSGeometry
from django.db.models.expressions import Value

class Person(models.Model):
    # Create this field without srid=None for the sake of creating
    # a working migration.
    home = models.PointField(null=True)


def run():
    # Because Django migrated this model, restore the model field
    # to have srid=None, to simulate an unmanaged model.
    home = Person._meta.get_fields()[1]
    home.srid = None

    me = Person.objects.create()
    me.home = GEOSGeometry("POINT(1 1)", srid=4326)
    me.save()
    me.refresh_from_db()
    assert me.home  # raises

Rerunning that snippet on 6.0 gives a DatabaseError instead. We can raise an error in the ORM layer instead.


[0] At least on PostGIS, because of a %d placeholder. I can check other backends shortly.

Change History (6)

in reply to:  description comment:1 by Jacob Walls, 4 weeks ago

[0] At least on PostGIS, because of a %d placeholder. I can check other backends shortly.

MySQL/MariaDB: a Django migration to create this column works, but transforms aren't supported, so never attempted
Spatialite: can't create the column: "django.db.utils.OperationalError: no such column: None"
Oracle: can't create the column: "django.db.utils.DatabaseError: ORA-00984: column not allowed here

comment:2 by Jacob Walls, 4 weeks ago

Summary: Saving to SpatialField(srid=None) persists NULL instead of raising DatabaseError → Saving to SpatialField(srid=None) persists NULL instead of raising an error

comment:3 by Yassin Bahri, 4 weeks ago

Owner: set to Yassin Bahri
Status: new → assigned
Triage Stage: Unreviewed → Accepted

I reproduced this using the linked DryORM PostGIS example.

  • With Django 6.0, me.save() raises ProgrammingError because the target None SRID is included as invalid SQL.
  • With Django 6.1, the save completes, but refresh_from_db() returns me.home as None.

This confirms a regression with silent data loss rather than only a change in the exception being raised.

I also checked the relevant change in 1a8fd5cf75bf855852f6bc2f75c3da9f7b145669. The target field SRID is now passed as a query parameter, so ST_Transform(..., NULL) returns NULL. The same code path remains on main.

The GeoDjango documentation describes a spatial field SRID as an integer specifier, so srid=None isn't a valid model field target SRID.

I agree with accepting this as a release-blocking regression. The fix should raise a clear Python exception before compiling a transform with a None target SRID. Regression coverage should include both a directly assigned geometry and the expression/output-field path, while preserving the existing srid=-1 behavior.

comment:4 by Yassin Bahri, 4 weeks ago

Has patch: set

comment:5 by Jacob Walls, 3 weeks ago

Patch needs improvement: set

comment:6 by Jacob Walls, 28 hours ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin
Note: See TracTickets for help on using tickets.
Back to Top