[postgis-tickets] r16969 - Revert change to avoid slicing on box access.

[Paul Ramsey]

Author: pramsey
Date: 2018-10-31 15:02:55 -0700 (Wed, 31 Oct 2018)
New Revision: 16969

Modified:
   branches/2.4/liblwgeom/g_serialized.c
   branches/2.4/postgis/gserialized_gist_2d.c
Log:
Revert change to avoid slicing on box access.
References #4216


Modified: branches/2.4/liblwgeom/g_serialized.c
===================================================================
--- branches/2.4/liblwgeom/g_serialized.c	2018-10-31 03:25:23 UTC (rev 16968)
+++ branches/2.4/liblwgeom/g_serialized.c	2018-10-31 22:02:55 UTC (rev 16969)
@@ -86,6 +86,7 @@
 uint32_t gserialized_get_type(const GSERIALIZED *s)
 {
 	uint32_t *ptr;
+	assert(s);
 	ptr = (uint32_t*)(s->data);
 	LWDEBUG(4,"entered");
 	if ( FLAGS_GET_BBOX(s->flags) )

Modified: branches/2.4/postgis/gserialized_gist_2d.c
===================================================================
--- branches/2.4/postgis/gserialized_gist_2d.c	2018-10-31 03:25:23 UTC (rev 16968)
+++ branches/2.4/postgis/gserialized_gist_2d.c	2018-10-31 22:02:55 UTC (rev 16969)
@@ -593,15 +593,18 @@
 	POSTGIS_DEBUG(4, "entered function");
 
 	/*
-	** Because geometry is declared as "storage = main" anything large
-	** enough to take serious advantage of PG_DETOAST_DATUM_SLICE will have
-	** already been compressed, which means the entire object will be
-	** fetched and decompressed before a slice is taken, thus removing
-	** any efficiencies gained from slicing. We need to move to
-	** "storage = external" and implement our own geometry compressor
-	** before we can take advantage of sliced retrieval.
+	** The most info we need is the 8 bytes of serialized header plus the
+	** of floats necessary to hold the bounding box.
 	*/
-	gpart = (GSERIALIZED*)PG_DETOAST_DATUM(gsdatum);
+	if (VARATT_IS_EXTENDED(gsdatum))
+	{
+		gpart = (GSERIALIZED*)PG_DETOAST_DATUM_SLICE(gsdatum, 0, 8 + sizeof(BOX2DF));
+	}
+	else
+	{
+		gpart = (GSERIALIZED*)PG_DETOAST_DATUM(gsdatum);
+	}
+
 	flags = gpart->flags;
 
 	POSTGIS_DEBUGF(4, "got flags %d", gpart->flags);
@@ -618,22 +621,28 @@
 	{
 		/* No, we need to calculate it from the full object. */
 		GBOX gbox;
+		GSERIALIZED *g = (GSERIALIZED*)PG_DETOAST_DATUM(gsdatum);
+
 		gbox_init(&gbox);
 
-		result = gserialized_get_gbox_p(gpart, &gbox);
-		if ( result == LW_SUCCESS )
+		if (gserialized_get_gbox_p(g, &gbox) == LW_FAILURE)
 		{
-			result = box2df_from_gbox_p(&gbox, box2df);
+			POSTGIS_DEBUG(4, "could not calculate bbox, returning failure");
+			POSTGIS_FREE_IF_COPY_P(gpart, gsdatum);
+			POSTGIS_FREE_IF_COPY_P(g, gsdatum);
+			return LW_FAILURE;
 		}
-		else
-		{
-			POSTGIS_DEBUG(4, "could not calculate bbox");
-		}
+		POSTGIS_FREE_IF_COPY_P(g, gsdatum);
+		result = box2df_from_gbox_p(&gbox, box2df);
 	}
 
 	POSTGIS_FREE_IF_COPY_P(gpart, gsdatum);
-	POSTGIS_DEBUGF(4, "result = %d, got box2df %s", result, result == LW_SUCCESS ? box2df_to_string(box2df) : "NONE");
 
+	if ( result == LW_SUCCESS )
+	{
+		POSTGIS_DEBUGF(4, "got box2df %s", box2df_to_string(box2df));
+	}
+
 	return result;
 }