Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19058 closed Bug (fixed)

Inserting NULL value in Oracle spatial backend causes crash.

Reported by: jtiai Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: gis oracle orm
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Seem to be regression of #10888.

Main issue is that when inserting or updating spatial field value cx_Oracle interpreted NULL value for SDO_GEOMETRY field when placeholder (%s) as a CHAR. (since it's Oracle user type actually). Both insert and update failed.

As a solution it was implemented so that when ever encountering None value for geometry field placeholder and actual value was removed from the whole insert clause.

Change History (8)

comment:1 Changed 3 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Marking this as accepted based on IRC discussions. I don't have spatial Oracle available, so I am relying on reporting alone.

comment:2 Changed 3 years ago by akaariai

First, there is a way to install Oracle "full edition" with spatial support using https://code.djangoproject.com/wiki/OracleSpatialTestSetup

The current proposal is to alter the code around line 890 in sql/compiler.py from:

        else:
            placeholders = [
                [self.placeholder(field, v) for field, v in zip(fields, val)]
                for val in values
            ]

to

        else:
            placeholders = [
                [self.placeholder(field, v) for field, v in zip(fields, val)]
                for val in values
            ]
            # Oracle GIS needs to remove some values due to #10888
            values = connection.ops.modify_insert_values(placeholders, values) 

Default implementation of modify_insert_values is to just return values, Oracle gis would need to loop the placeholders and values in sync and remove values for 'NULL' placeholders.

This way we could fix this issue, and also get rid of Oracle GIS compiler.py.

comment:3 Changed 3 years ago by jtiai

Initial implementation as discussed above:

https://github.com/jtiai/django/commit/a5ebc25

There is still quite a bunch of tests failing on Oracle.

comment:4 Changed 3 years ago by akaariai

I am going to push the patch pretty much as is. Even if Oracle GIS is still broken after this to me it seems like one step in the right direction. We can't break Oracle GIS, and I can't see how this could break anything else. So, I will push this to 1.4.x on the grounds that this is a crash-fix for Oracle GIS.

comment:5 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 92d7f541da8b59520c833b19fbba52d3ecef2428:

Fixed #19058 -- Fixed Oracle GIS crash

The problem is the same as in #10888 which was reintroduced when
bulk_insert was added. Thanks to Jani Tiainen for report, patch and
also testing the final patch on Oracle GIS.

comment:6 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

In 33f1181c31e0ee5bc5e20ca2a8549508de807623:

[1.5.x] Fixed #19058 -- Fixed Oracle GIS crash

The problem is the same as in #10888 which was reintroduced when
bulk_insert was added. Thanks to Jani Tiainen for report, patch and
also testing the final patch on Oracle GIS.

Backpatch of 92d7f541da8b59520c833b19fbba52d3ecef2428

comment:7 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

In 25e041f270bec5a69e525d47f18a73fe917d1787:

[1.4.x] Fixed #19058 -- Fixed Oracle GIS crash

The problem is the same as in #10888 which was reintroduced when
bulk_insert was added. Thanks to Jani Tiainen for report, patch and
also testing the final patch on Oracle GIS.

Backpatch of 92d7f541da8b59520c833b19fbba52d3ecef2428

comment:8 Changed 3 years ago by Preston Holmes <preston@…>

In 7e71c7deac1f189c5c11077c03fd53ffb23d99e4:

Fixed #19058 -- Fixed Oracle GIS crash

The problem is the same as in #10888 which was reintroduced when
bulk_insert was added. Thanks to Jani Tiainen for report, patch and
also testing the final patch on Oracle GIS.

Note: See TracTickets for help on using tickets.
Back to Top