Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed GIS tests with Oracle 23c. #17724

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

smithdc1
Copy link
Member

My attempt at a patch to allow GIS tests to pass with Oracle 23c.

The change to LayerMapping is due to Oracle 23c changing "MULTI" geometries containing one value to their single type.

create table LAYERMAP_COUNTY_TEST
(
    ID       NUMBER(11),
    NAME     NVARCHAR2(25),
    STATE_ID NUMBER(11),
    MPOLY    MDSYS.SDO_GEOMETRY
);

INSERT INTO "LAYERMAP_COUNTY_TEST" ("NAME",
                               "STATE_ID",
                               "MPOLY")
VALUES ('Galveston', 3, SDO_GEOMETRY('MULTIPOLYGON (((30 20, 45 40, 10 40, 30 20)))',4269));

INSERT INTO "LAYERMAP_COUNTY_TEST" ("NAME",
                               "STATE_ID",
                               "MPOLY")
VALUES ('Hawaii', 4, SDO_GEOMETRY('MULTIPOLYGON (((30 20, 45 40, 10 40, 30 20)),
((15 5, 40 10, 10 20, 5 10, 15 5)))',4269));

select SDO_UTIL.TO_WKTGEOMETRY("LAYERMAP_COUNTY_TEST"."MPOLY")
 from LAYERMAP_COUNTY_TEST;

"POLYGON ((30.0 20.0, 45.0 40.0, 10.0 40.0, 30.0 20.0))"
"MULTIPOLYGON (((30.0 20.0, 45.0 40.0, 10.0 40.0, 30.0 20.0)), ((15.0 5.0, 40.0 10.0, 10.0 20.0, 5.0 10.0, 15.0 5.0)))"

@smithdc1 smithdc1 marked this pull request as draft January 11, 2024 20:34
)
if connection.ops.oracle and connection.oracle_version >= (23,):
forney_houston = GEOSGeometry(
"MULTIPOINT (-95.363151 29.763374, -96.801611 32.782057)",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this PR as draft.

This is the edit to make the test pass, but it seems wrong to me. This new point is Dallas (entering the point at https://www.geometrymapper.com/), before it was Forney. Note above would suggest this change is wrong.

Thoughts welcome 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's related with undefined ordering 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking another look at this morning. Just leading a small note here for now to say that I've just realised the supports_tolerance_parameter feature (and therefore this test) is only enabled on Oracle.

Oracle 23c returns Points with an increased level of precision. Adjusted test to round WKT to a set level of decimal places.
Oracle 23c collapses MULTI geometries with only one entry to their non-MULTI type. Added check when loading geometries from the database that a MULTI type is returned. Reconvert to correct geometry type, if required.

wkt_w = WKTWriter(trim=True, precision=6)
self.assertEqual(
{wkt_w.write(p).decode("utf-8") for p in ref_u1},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suspecting these test failures are due to version differences in GDAL/GEOS.

There's equals_exact() which accepts a tolerance. That would be useful here, but what's tricky here is that we're comparing two sets. I'm not sure the Points can be ordered in a deterministic(?) way first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants