[postgis-tickets] r16059 - Equality test failed on points with same coordinates

[Paul Ramsey]

Author: pramsey
Date: 2017-10-25 06:26:11 -0700 (Wed, 25 Oct 2017)
New Revision: 16059

Modified:
   trunk/liblwgeom/g_serialized.c
   trunk/postgis/lwgeom_btree.c
   trunk/regress/tickets.sql
   trunk/regress/tickets_expected
Log:
Equality test failed on points with same coordinates
and differing SRIDs, resulting in incorrect hashing. 
Amazingly, this problem existed in *both* the btree
function and the new hash machinery (and it was the
btree machinery that was actually called for the 
UNION operation).
References #1014


Modified: trunk/liblwgeom/g_serialized.c
===================================================================
--- trunk/liblwgeom/g_serialized.c	2017-10-24 21:25:45 UTC (rev 16058)
+++ trunk/liblwgeom/g_serialized.c	2017-10-25 13:26:11 UTC (rev 16059)
@@ -305,10 +305,10 @@
 	if (
 		sz1 > 16 && // 16 is size of EMPTY, if it's larger - it has coordinates
 		sz2 > 16 &&
-		*(uint32_t*)(g1 + 2 * sizeof(uint32_t)) == POINTTYPE &&
-		*(uint32_t*)(g2 + 2 * sizeof(uint32_t)) == POINTTYPE &&
 		!FLAGS_GET_BBOX(g1->flags) &&
-		!FLAGS_GET_BBOX(g2->flags)
+		!FLAGS_GET_BBOX(g2->flags) &&
+		*(uint32_t*)(g1->data) == POINTTYPE &&
+		*(uint32_t*)(g2->data) == POINTTYPE
 	)
 	{
 		double *dptr = (double*)(g1->data + sizeof(double));
@@ -321,12 +321,18 @@
 		y.f = 2.0 * dptr[1];
 		hash2 = uint32_interleave_2(x.u, y.u);
 
-		if ( hash1 > hash2 )
-			return 1;
-		if ( hash1 < hash2 )
-			return -1;
+		/* If the SRIDs are the same, we can use hash inequality */
+		/* to jump us out of this function early. Otherwise we still */
+		/* have to do the full calculation */
+		if ( gserialized_cmp_srid(g1, g2) == 0 )
+		{
+			if ( hash1 > hash2 )
+				return 1;
+			if ( hash1 < hash2 )
+				return -1;
+		}
 
-		// if hashes happen to be the same, go to full compare.
+		/* if hashes happen to be the same, go to full compare. */
 	}
 
 	size_t hsz1 = gserialized_header_size(g1);
@@ -401,7 +407,7 @@
 		else if (bsz1 > bsz2)
  			return 1;
 	}
-	return cmp == 0 ? 0 : (cmp > 0 ? 1 : -1);
+	return cmp > 0 ? 1 : -1;
 }
 
 int gserialized_read_gbox_p(const GSERIALIZED *g, GBOX *gbox)

Modified: trunk/postgis/lwgeom_btree.c
===================================================================
--- trunk/postgis/lwgeom_btree.c	2017-10-24 21:25:45 UTC (rev 16058)
+++ trunk/postgis/lwgeom_btree.c	2017-10-25 13:26:11 UTC (rev 16059)
@@ -132,11 +132,23 @@
 Datum lwgeom_hash(PG_FUNCTION_ARGS)
 {
 	GSERIALIZED *g1 = PG_GETARG_GSERIALIZED_P(0);
-	size_t sz1 = VARSIZE(g1);
+	/* Point to just the type/coordinate part of buffer */
 	size_t hsz1 = gserialized_header_size(g1);
 	uint8_t *b1 = (uint8_t*)g1 + hsz1;
+	/* Calculate size of type/coordinate buffer */
+	size_t sz1 = VARSIZE(g1);
 	size_t bsz1 = sz1 - hsz1;
-	Datum hval = hash_any(b1, bsz1);
+	/* Calculate size of srid/type/coordinate buffer */
+	int srid = gserialized_get_srid(g1);
+	size_t bsz2 = bsz1 + sizeof(int);
+	uint8_t *b2 = palloc(bsz2);
+	/* Copy srid into front of combined buffer */
+	memcpy(b2, &srid, sizeof(int));
+	/* Copy type/coordinates into rest of combined buffer */
+	memcpy(b2+sizeof(int), b1, bsz1);
+	/* Hash combined buffer */
+	Datum hval = hash_any(b2, bsz2);
+	pfree(b2);
 	PG_FREE_IF_COPY(g1, 0);
 	PG_RETURN_DATUM(hval);
 }

Modified: trunk/regress/tickets.sql
===================================================================
--- trunk/regress/tickets.sql	2017-10-24 21:25:45 UTC (rev 16058)
+++ trunk/regress/tickets.sql	2017-10-25 13:26:11 UTC (rev 16059)
@@ -1048,6 +1048,11 @@
     WHERE ST_Y(path.g) = ST_X(rec.g)
 )
 SELECT '#1014c', id, st_astext(g) FROM path;
+SELECT '#1014d', ST_AsEWKT(g) FROM (
+	SELECT 'SRID=1;POINT(0 1)'::geometry AS g
+	UNION
+	SELECT 'SRID=2;POINT(0 1)'::geometry AS g
+) a;
 DROP TABLE IF EXISTS rec;
 
 

Modified: trunk/regress/tickets_expected
===================================================================
--- trunk/regress/tickets_expected	2017-10-24 21:25:45 UTC (rev 16058)
+++ trunk/regress/tickets_expected	2017-10-25 13:26:11 UTC (rev 16059)
@@ -312,3 +312,5 @@
 #1014c|1|POINT(0 1)
 #1014c|2|POINT(1 2)
 #1014c|3|POINT(2 3)
+#1014d|SRID=1;POINT(0 1)
+#1014d|SRID=2;POINT(0 1)