Opened 5 years ago

Closed 5 years ago

#4608 closed defect (fixed)

Geography GIST index fails on ST_Covers

Reported by: jgh Owned by: pramsey
Priority: high Milestone: PostGIS 3.0.1
Component: postgis Version: 3.0.x
Keywords: Cc:

Description

st_contains relying on a GIST index on a geography column fails if the table is named public.test.

It seems to work fine with another table name OR another schema.

It was observed with st_contains but could affect other functions, though at least st_intersects works fine.

The issue was first reported here: https://gis.stackexchange.com/questions/344408/st-covers-fails-on-geography-column-with-gist-index-postgis-3-0

select postgis_full_version();

POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="120" GEOS="3.8.0-CAPI-1.13.1 " PROJ="Rel. 5.2.0, September 15th, 2018" LIBXML="2.9.9" LIBJSON="0.12" LIBPROTOBUF="1.2.1" WAGYU="0.4.3 (Internal)"


CREATE TABLE test (id SERIAL, the_geog GEOGRAPHY);

CREATE INDEX test_geog_idx ON test USING GIST(the_geog);

SELECT id FROM test WHERE ST_Covers(the_geog, ST_GeogFromText('POINT(-0.1 51.5)'));

ERROR: no spatial operator found for opfamily 118328 strategy 7


CREATE TABLE test1 (id SERIAL, the_geog GEOGRAPHY);

CREATE INDEX test_geog_idx ON test USING GIST(the_geog);

SELECT id FROM test1 WHERE ST_Covers(the_geog, ST_GeogFromText('POINT(-0.1 51.5)'));

id

(0 row)

While the impact on a production server is almost nul, having a public.test table is fairly common so this issue could affect a large amount of users.

Change History (11)

comment:1 by robe, 5 years ago

Wow that's a weird one. I was able to replicate on my instance:

PostgreSQL 12.1, compiled by Visual C++ build 1914, 64-bit POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="120" GEOS="3.8.0-CAPI-1.13.1 " PROJ="Rel. 5.2.0, September 15th, 2018" GDAL="GDAL 2.4.3, released 2019/10/28" LIBXML="2.9.9" LIBJSON="0.12" LIBPROTOBUF="1.2.1" WAGYU="0.4.3 (Internal)" TOPOLOGY RASTER
ERROR:  no spatial operator found for opfamily 51535 strategy 7
SQL state: XX000

and on my

PostgreSQL 12.1 (Ubuntu 12.1-1.pgdg18.04+1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0, 64-bit POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="120" GEOS="3.7.1-CAPI-1.11.1 27a5e771" PROJ="Rel. 4.9.3, 15 August 2016" LIBXML="2.9.4" LIBJSON="0.12.1" LIBPROTOBUF="1.2.1" WAGYU="0.4.3 (Internal)" (core procs from "3.0.0rc1 r17886" need upgrade)

I get:

ERROR:  no spatial operator found for opfamily 19319 strategy 7

so it seems the opfamily is probably tied to the table oid?

Last edited 5 years ago by robe (previous) (diff)

comment:2 by robe, 5 years ago

Summary: Geography GIST index fails on table named testGeography GIST index fails on ST_Covers (table named test)

Okay I don't think this has anything to do with the table name.

I have the same issue with any geography table I have when using ST_Covers.

Just tried on my airports table

SELECT * from ch09.airports WHERE ST_Covers(geog, ST_GeogFromText('POINT(-0.1 51.5)'));

and get error

ERROR:  no spatial operator found for opfamily 51535 strategy 7
SQL state: XX000

But replacing with ST_Intersects works. So I think it's our ST_Covers operator stuff needs some loving.

comment:3 by Algunenano, 5 years ago

Milestone: PostGIS 3.1.0PostGIS 3.0.1
Priority: lowhigh

Moving this to 3.0.1 since it affects PG12 + Postgis 3.0.

ST_Covers index function is declared as:

	{"st_covers", RTContainsStrategyNumber, 2, 0},

Which is declared as:

#define RTContainsStrategyNumber		7	/* for @> */

Then gist-geometry is declared as:

CREATE OPERATOR CLASS gist_geography_ops
	DEFAULT FOR TYPE geography USING GIST AS
	STORAGE 	gidx,
	OPERATOR        3        &&	,
--	OPERATOR        6        ~=	,
--	OPERATOR        7        ~	,
--	OPERATOR        8        @	,

i.e. no strategy 7 / operator ~, hence the error you are seeing.

By looking at the old (pre PG12) declaration of ST_Covers(geography, geography) I see that it was using the operator && instead:

CREATE OR REPLACE FUNCTION ST_Covers(geog1 geography, geog2 geography)
	RETURNS boolean
	AS 'SELECT $1 OPERATOR(@extschema@.&&) $2 AND @extschema@._ST_Covers($1, $2)'
	LANGUAGE 'sql' IMMUTABLE PARALLEL SAFE;

So we might need to add a special case for this or have 2 arrays of support functions, one for geometry and one for geography. What do you think @pramsey?

comment:4 by jgh, 5 years ago

Thanks for looking at this issue.

It was my mistake for the table name: the table name in the 2nd statement, for the index creation, was not updated to the new table name, so the query on the index-less table was OK.

comment:5 by robe, 5 years ago

Summary: Geography GIST index fails on ST_Covers (table named test)Geography GIST index fails on ST_Covers

taking out the table name test on the subject since it fails for all geography tables with an index

comment:6 by Algunenano, 5 years ago

Some notes from reviewing this in detail:

  • Geometry:
    {"st_containsproperly", RTOverlapStrategyNumber, 2, 0},
    
    This doesn't match the old implementation and it should use RTContainsStrategyNumber: SELECT $1 OPERATOR(@extschema@.~) $2 AND @extschema@._ST_ContainsProperly($1,$2)'.
  • Geography:
    • ST_Contains, ST_Within, ST_Touches, ST_3dIntersects (and some more) don't exist for geography.This isn't an issue, but I'm going to use RTMaxStrategyNumber for those cases to make it clearer in the implementation.
    • ST_CoveredBy is implemented as SELECT $1 OPERATOR(@extschema@.&&) $2 AND @extschema@._ST_Covers($2, $1)' in prePG12, but as geography_coveredby in PG12. I think we should unify them to avoid confusion and always use geography_coveredby. Also, it's using RTContainedByStrategyNumber but it should use RTOverlapStrategyNumber for geography.
    • ST_Covers: As mentioned in #comment:3, it should also use RTOverlapStrategyNumber.

To fix these 3 issues I plan to create 2 different strategy arrays, one for geometry and one for geography and choose based on the input, so it should be easy to add a new one (raster for example) when the time is right.

comment:7 by Algunenano, 5 years ago

A couple of other issues:

Version 0, edited 5 years ago by Algunenano (next)

comment:8 by Algunenano, 5 years ago

WIP PR: https://github.com/postgis/postgis/pull/528 Pending adding tests to check the fixes

comment:9 by Algunenano, 5 years ago

I've added the tests for ST_Covers and ST_CoveredBy.

I don't see an easy way to test the change in st_containsproperly since the bug (it was using && instead of ~) doesn't have an impact in the results; all cases where a ~ b are included in a && b (but not the other way around) except when b is EMPTY , but then containsproperly discards empty geometries. I'm not aware of any way to check whether the result comes from the index operator or the C function itself.

comment:10 by Raúl Marín <git@…>, 5 years ago

In 64d4a1a/git:

PG12: Fix several bugs in the index support function

  • ST_Covers and ST_CoveredBy for geography were using the wrong strategy (which didn't exist).
  • ST_ContainsProperly for geometry was using the wrong strategy.
  • ST_3DIntersects was duplicated.

References #4608
Closes https://github.com/postgis/postgis/pull/528

comment:11 by Raúl Marín <git@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 36bcfa1/git:

PG12: Fix several bugs in the index support function

  • ST_Covers and ST_CoveredBy for geography were using the wrong strategy (which didn't exist).
  • ST_ContainsProperly for geometry was using the wrong strategy.
  • ST_3DIntersects was duplicated.

Closes #4608

Note: See TracTickets for help on using tickets.