[postgis-tickets] r16825 - St_AsMVT: Fix id support, and extra tests and documentation

Raul raul at rmr.ninja
Mon Sep 17 04:15:50 PDT 2018


Author: algunenano
Date: 2018-09-17 04:15:50 -0700 (Mon, 17 Sep 2018)
New Revision: 16825

Modified:
   trunk/doc/reference_output.xml
   trunk/postgis/lwgeom_out_mvt.c
   trunk/postgis/mvt.c
   trunk/postgis/mvt.h
   trunk/regress/mvt.sql
   trunk/regress/mvt_expected
Log:
St_AsMVT: Fix id support, and extra tests and documentation

Closes #4128
Closes https://github.com/postgis/postgis/pull/303


Modified: trunk/doc/reference_output.xml
===================================================================
--- trunk/doc/reference_output.xml	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/doc/reference_output.xml	2018-09-17 11:15:50 UTC (rev 16825)
@@ -1487,7 +1487,7 @@
 		<para><varname>name</varname> is the name of the Layer. If NULL it will use the string "default".</para>
 		<para><varname>extent</varname> is the tile extent in screen space as defined by the specification. If NULL it will default to 4096.</para>
 		<para><varname>geom_name</varname> is the name of the geometry column in the row data. If NULL it will default to the first found geometry column.</para>
-		<para><varname>feature_id_name</varname> is the name of the Feature ID column in the row data. If NULL Feature ID is not set.</para>
+		<para><varname>feature_id_name</varname> is the name of the Feature ID column in the row data. If NULL or negative the Feature ID is not set. The first column matching name and valid type (smallint, integer, bigint) will be used as Feature ID, and any subsequent column will be added as a property. JSON properties are not supported.</para>
 
 
 	    <para>Enhanced: 3.0 - added support for Feature ID.</para>

Modified: trunk/postgis/lwgeom_out_mvt.c
===================================================================
--- trunk/postgis/lwgeom_out_mvt.c	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/postgis/lwgeom_out_mvt.c	2018-09-17 11:15:50 UTC (rev 16825)
@@ -102,6 +102,8 @@
 			ctx->geom_name = text_to_cstring(PG_GETARG_TEXT_P(4));
 		if (PG_NARGS() > 5 && !PG_ARGISNULL(5))
 			ctx->id_name = text_to_cstring(PG_GETARG_TEXT_P(5));
+		else
+			ctx->id_name = NULL;
 		mvt_agg_init_context(ctx);
 	} else {
 		ctx = (mvt_agg_context *) PG_GETARG_POINTER(0);

Modified: trunk/postgis/mvt.c
===================================================================
--- trunk/postgis/mvt.c	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/postgis/mvt.c	2018-09-17 11:15:50 UTC (rev 16825)
@@ -22,6 +22,8 @@
  *
  **********************************************************************/
 
+#include <string.h>
+
 #include "mvt.h"
 
 #ifdef HAVE_LIBPROTOBUF
@@ -368,11 +370,24 @@
 			}
 		}
 
-		ctx->column_cache.column_keys_index[i] = add_key(ctx, pstrdup(tkey));
+		if (ctx->id_name &&
+			(ctx->id_index == UINT32_MAX) &&
+			(strcmp(tkey, ctx->id_name) == 0) &&
+			(typoid == INT2OID || typoid == INT4OID || typoid == INT8OID))
+		{
+			ctx->id_index = i;
+		}
+		else
+		{
+			ctx->column_cache.column_keys_index[i] = add_key(ctx, pstrdup(tkey));
+		}
 	}
 
 	if (!geom_found)
 		elog(ERROR, "parse_column_keys: no geometry column found");
+
+	if (ctx->id_name != NULL && ctx->id_index == UINT32_MAX)
+		elog(ERROR, "mvt_agg_transfn: Could not find column '%s' of integer type", ctx->id_name);
 }
 
 static void encode_keys(mvt_agg_context *ctx)
@@ -626,10 +641,43 @@
 }
 #endif
 
-static void set_feature_id(mvt_agg_context *ctx, Datum datum)
+/**
+ * Sets the feature id. Ignores Nulls and negative values
+ */
+static void set_feature_id(mvt_agg_context *ctx, Datum datum, bool isNull)
 {
-	ctx->feature->id = datum;
+	Oid typoid = ctx->column_cache.column_oid[ctx->id_index];
+	int64_t value = INT64_MIN;
+
+	if (isNull)
+	{
+		POSTGIS_DEBUG(3, "set_feature_id: Ignored null value");
+		return;
+	}
+
+	switch (typoid)
+	{
+	case INT2OID:
+		value = DatumGetInt16(datum);
+		break;
+	case INT4OID:
+		value = DatumGetInt32(datum);
+		break;
+	case INT8OID:
+		value = DatumGetInt64(datum);
+		break;
+	default:
+		elog(ERROR, "set_feature_id: Feature id type does not match");
+	}
+
+	if (value < 0)
+	{
+		POSTGIS_DEBUG(3, "set_feature_id: Ignored negative value");
+		return;
+	}
+
 	ctx->feature->has_id = true;
+	ctx->feature->id = (uint64_t) value;
 }
 
 static void parse_values(mvt_agg_context *ctx)
@@ -666,6 +714,12 @@
 		if (i == ctx->geom_index)
 			continue;
 
+		if (i == ctx->id_index)
+		{
+			set_feature_id(ctx, datum, cc.nulls[i]);
+			continue;
+		}
+
 		if (cc.nulls[i])
 		{
 			POSTGIS_DEBUG(3, "parse_values isnull detected");
@@ -724,10 +778,6 @@
 			break;
 		}
 
-		if (ctx->id_name != NULL && strcmp(key, ctx->id_name) == 0 && (typoid == INT2OID || typoid == INT4OID || typoid == INT8OID)) {
-			set_feature_id(ctx, datum);
-		}
-
 		ctx->row_columns++;
 	}
 
@@ -916,6 +966,7 @@
 	ctx->bool_values_hash = NULL;
 	ctx->values_hash_i = 0;
 	ctx->keys_hash_i = 0;
+	ctx->id_index = UINT32_MAX;
 	ctx->geom_index = UINT32_MAX;
 
 	memset(&ctx->column_cache, 0, sizeof(ctx->column_cache));

Modified: trunk/postgis/mvt.h
===================================================================
--- trunk/postgis/mvt.h	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/postgis/mvt.h	2018-09-17 11:15:50 UTC (rev 16825)
@@ -60,6 +60,7 @@
 	char *name;
 	uint32_t extent;
 	char *id_name;
+	uint32_t id_index;
 	char *geom_name;
 	uint32_t geom_index;
 	HeapTupleHeader row;

Modified: trunk/regress/mvt.sql
===================================================================
--- trunk/regress/mvt.sql	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/regress/mvt.sql	2018-09-17 11:15:50 UTC (rev 16825)
@@ -385,11 +385,6 @@
 			ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
 ) AS q;
 
--- feature id encoding tests
-SELECT 'FI1', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (SELECT 1 AS c1, 'abcd'::text AS c2,
-	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
-		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom) AS q;
-
 -- default values tests
 SELECT 'D1', encode(ST_AsMVT(q, 'test', 4096, 'geom'), 'base64') FROM (SELECT 1 AS c1, 'abcd'::text AS c2,
 	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
@@ -430,3 +425,64 @@
 		true
 		)));
 
+
+-- Feature id encoding tests
+SELECT 'FI1', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 1::smallint AS c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+SELECT 'FI2', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 1::integer AS c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+SELECT 'FI3', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 1::bigint AS c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- Column not found
+SELECT 'FI4', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- Column of invalid type
+SELECT 'FI5', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 2.0 as c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- Null value is ignored
+SELECT 'FI6', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT NULL::integer as c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- Negative value is ignored
+SELECT 'FI7', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT -5::integer as c1, 'abcd'::text AS c2,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- When the column is repeated, the fist one is used as id and the second one is added as a property
+SELECT 'FI8', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 1::smallint AS c1, 'abcd'::text AS c2, 20::integer as c1,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
+
+-- When the column is repeated: Only the ones with the valid types are considered
+SELECT 'FI9', encode(ST_AsMVT(q, 'test', 4096, 'geom', 'c1'), 'base64') FROM (
+	SELECT 1::double precision AS c1, 'abcd'::text AS c2, 20::integer as c1,
+	ST_AsMVTGeom(ST_GeomFromText('POINT(25 17)'),
+		ST_MakeBox2D(ST_Point(0, 0), ST_Point(4096, 4096)), 4096, 0, false) AS geom
+) AS q;
\ No newline at end of file

Modified: trunk/regress/mvt_expected
===================================================================
--- trunk/regress/mvt_expected	2018-09-17 05:06:17 UTC (rev 16824)
+++ trunk/regress/mvt_expected	2018-09-17 11:15:50 UTC (rev 16825)
@@ -85,7 +85,6 @@
 NzJBODE2NTQ0MUQ5NDBERkJBRkQ5RUYyNDA0NzRENjI3MkE4MTY1NDQxKIAgeAI=
 TA15|GkkKBHRlc3QSEBIGAAABAQIAGAEiBAky3j8aAmMxGgJjMhoHY3N0cmluZyIDCgExIhQKEjEyLjIz
 MjM4OTI4MzIyMzIzOSiAIHgC
-FI1|GjEKBHRlc3QSEAgBEgQAAAEBGAEiBAky3j8aAmMxGgJjMiICKAEiBgoEYWJjZCiAIHgC
 D1|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
 D2|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
 D3|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
@@ -97,3 +96,13 @@
 ERROR:  pgis_asmvt_transfn: parameter row cannot be other than a rowtype
 TU3|
 #3922|91
+FI1|GicKBHRlc3QSDggBEgIAABgBIgQJMt4/GgJjMiIGCgRhYmNkKIAgeAI=
+FI2|GicKBHRlc3QSDggBEgIAABgBIgQJMt4/GgJjMiIGCgRhYmNkKIAgeAI=
+FI3|GicKBHRlc3QSDggBEgIAABgBIgQJMt4/GgJjMiIGCgRhYmNkKIAgeAI=
+ERROR:  mvt_agg_transfn: Could not find column 'c1' of integer type
+ERROR:  mvt_agg_transfn: Could not find column 'c1' of integer type
+FI6|GiUKBHRlc3QSDBICAAAYASIECTLePxoCYzIiBgoEYWJjZCiAIHgC
+FI7|GiUKBHRlc3QSDBICAAAYASIECTLePxoCYzIiBgoEYWJjZCiAIHgC
+FI8|GjEKBHRlc3QSEAgBEgQAAAEBGAEiBAky3j8aAmMyGgJjMSIGCgRhYmNkIgIoFCiAIHgC
+FI9|GjgKBHRlc3QSEAgUEgQAAAEBGAEiBAky3j8aAmMxGgJjMiIJGQAAAAAAAPA/IgYKBGFiY2QogCB4
+Ag==



More information about the postgis-tickets mailing list