CI/TST: Use same SRIDs for test_geo_spatial_binops

[semelianova]

Different SRIDs aren't supported in Omnisci backend since commit . Exception is occurred. Can't use transform func separately to change SRID in the database table so create new objects with the same default SRID.

[semelianova]

It seems OmnisciDB and PostgreSQL set a different default SRID in the table. It is possible to define SRID in the table creation or use different points with suitable SRID for different backends. In the first case when I set 4326 SRID I get doubles instead of ints the table that occurs errors in several test suites.

[pep8speaks]

Hello @semelianova! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-02-12 06:41:36 UTC

[xmnlab]

@semelianova thanks for working on that!

In the first case when I set 4326 SRID I get doubles instead of ints the table that occurs errors in several test suites.

could you give me an example about that?

[semelianova]

I specifically define SRID

CREATE TABLE geo (
  id INTEGER,
  geo_point GEOMETRY(POINT, 4326),
  geo_linestring GEOMETRY(LINESTRING, 4326),
  geo_polygon GEOMETRY(POLYGON, 4326),
  geo_multipolygon GEOMETRY(MULTIPOLYGON, 4326)
);

in /ci/schema/omniscidb.sql then I load data by ci/datamgr.py

omnisql> SELECT * FROM geo;
output:

id|geo_point|geo_linestring|geo_polygon|geo_multipolygon
1|POINT (0 0)|LINESTRING (0 0,0.999999940861017 0.999999982770532)|
POLYGON(29.9999999860302 9.99999999534339,
39.9999999813735 39.9999999813735,
19.9999999906868 39.9999999813735,
9.99999999534339 19.9999999906868,
29.9999999860302 9.99999999534339))|
MULTIPOLYGON (((29.9999999860302 19.999999990686 8,
44.9999999371357 39.9999999813735,
9.99999999534339 39.9999999813735,
29.9999999860302 19.9999999906868)),
((14.9999999511056 4.99999999767169,
39.9999999813735 9.99999999534339,
9.99999999534339 19.9999999906868,
4.99999995576218.99999999534339,
14.9999999511056 4.99999999767169)))

...

While with default SRID I get:

id|geo_point|geo_linestring|geo_polygon|geo_multipolygon
1|POINT (0 0)|LINESTRING (0 0,1 1)|
POLYGON ((30 10,40 40,20 40,10 20,30 10))|
MULTIPOLYGON (((30 20,45 40,10 40,30 20)),((15 5,40 10,10 20,5 10,15 5)))

[xmnlab]

@semelianova thanks for caching that up. so maybe would be good to force srid 0 when there is no srid specified for omniscidb. how does it sounds to you?

[semelianova]

I have nothing against, it sounds good to me.

[anmyachev]

@semelianova pls merge master

[semelianova]

@xmnlab PR is ready for review

[xmnlab]

@semelianova thanks for ping me .. I was playing with your branch last week .. ... maybe we would need to change the approach ... but I need to play a little bit more ... I am going to play with that again today, sorry for the delay.

[xmnlab]

@semelianova I was checking this issue. it seems the default for postgresql is 4326 (https://postgis.net/docs/using_postgis_dbmanagement.html#Geography_Basics)

so probably we don't want to define default for postgresql to 0

so for your PR maybe you should just force 4326 for omniscidb in the table creation and change the test if necessary.

not sure if it is enough for every case when srid is omitted .. but at least for the test it would be enough .. and we can discuss more that in a follow up issue/PR (also we could propose a community meeting to discuss that)

how does it sounds?

[jreback]

looks ok to me, @xmnlab comments.

[semelianova]

Yes, OmnisciDB and PostgreSQL set a different default SRID in the table, 4326 is the default for PostgreSQL, 0 is for OmnisciDB. I can't understand why you want to save default 4326 SRID for PostgreSQL but force push 4326 to OmnisciDB. I can do this way but it is possible to occur difficulties with doubles (comparisons etc).

[xmnlab]

@semelianova I think for this PR we don't need to worry about the "default" and we can just create the geo table using the same SRID for both tables.

as literals without SRID (eg. points, linestring, etc), in theory, will be treated by each backend the same SRID used by the table creation, so it would be fine.

in general ibis tries to have an uniform behavior for the operations for all backends. for example window rank operations always starts from 0 in ibis land ... so if some backend starts from 1 ... we just add inside the translation -1.

not sure if we can do something similar to default SRID, because it is more complex than just adding -1 in the translation. it could have side effects.

so I think for now we don't need to worry about the default srid .. and just use the same SRID for each backend for geo table on CI and probably you will not have any side effect.

let me know if I am missing any point.

[semelianova]

I tried to force push 4326 SRID to omnisci, got double values in geo-coordinates, which aren't correctly compared that is a barrier for geo_contains function. It is possible a bug on omnisci side.

[xmnlab]

mmm interesting .. have you opened an issue on omniscidb repo?
we can accept for now the geo test table with srid 0 for both .. but I think that would be good to have a follow up issue for setting srid 4326 for both again when it is working on omniscidb. could you please could open an issue for that? thanks!

[xmnlab]

LGTM! thanks @semelianova for working on that!