St_AsMVT: Fix id support, and extra tests and documentation

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

git-svn-id: http://svn.osgeo.org/postgis/trunk@16825 b70326c6-7e19-0410-871a-916f4a2858ee

diff --git a/doc/reference_output.xml b/doc/reference_output.xml
index 2dd237350d5495aa57e04d60bc3456de35b57bad..09120e22356bf9a6d4bf82adfc186ce0a68f8134 100644 (file)
--- a/doc/reference_output.xml
+++ b/doc/reference_output.xml
@@ -1487,7 +1487,7 @@ SELECT ST_GeoHash(ST_SetSRID(ST_MakePoint(-126,48),4326),5);
                <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>

diff --git a/postgis/lwgeom_out_mvt.c b/postgis/lwgeom_out_mvt.c
index 06925f0cece6000d6068a35c27813fba1bb469b5..e2e05298eee8fbe70b5c9408c6cf7c859bc5a53e 100644 (file)
--- a/postgis/lwgeom_out_mvt.c
+++ b/postgis/lwgeom_out_mvt.c
@@ -102,6 +102,8 @@ Datum pgis_asmvt_transfn(PG_FUNCTION_ARGS)
                        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);

diff --git a/postgis/mvt.c b/postgis/mvt.c
index aec13fe47434b71bd42588ae55c0386754f2b966..9ebc61882fd7e605b9d930e9cd6c79a303b04fa5 100644 (file)
--- a/postgis/mvt.c
+++ b/postgis/mvt.c
@@ -22,6 +22,8 @@
  *
  **********************************************************************/
 
+#include <string.h>
+
 #include "mvt.h"
 
 #ifdef HAVE_LIBPROTOBUF
@@ -368,11 +370,24 @@ static void parse_column_keys(mvt_agg_context *ctx)
                        }
                }
 
-               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 @@ static uint32_t *parse_jsonb(mvt_agg_context *ctx, Jsonb *jb,
 }
 #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 @@ static void parse_values(mvt_agg_context *ctx)
                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 @@ static void parse_values(mvt_agg_context *ctx)
                        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 @@ void mvt_agg_init_context(mvt_agg_context *ctx)
        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));

diff --git a/postgis/mvt.h b/postgis/mvt.h
index 352931e2c4670d365a4f575ce33687c2d8e08c00..c596437ab6c10ec57f04b9b7dd914d666e227a8b 100644 (file)
--- a/postgis/mvt.h
+++ b/postgis/mvt.h
@@ -60,6 +60,7 @@ typedef struct mvt_agg_context
        char *name;
        uint32_t extent;
        char *id_name;
+       uint32_t id_index;
        char *geom_name;
        uint32_t geom_index;
        HeapTupleHeader row;

diff --git a/regress/mvt.sql b/regress/mvt.sql
index bc12c66a2b11db6b6a96498635a622a972926d40..7b9b4e222cdc823f4efbba8fb9c0dee3a4b9cce1 100644 (file)
--- a/regress/mvt.sql
+++ b/regress/mvt.sql
@@ -385,11 +385,6 @@ SELECT 'TA15', encode(ST_AsMVT(q, 'test', 4096, 'geom'), 'base64') FROM
                        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 @@ SELECT '#3922', length(bytea(ST_AsMVTGeom(
                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

diff --git a/regress/mvt_expected b/regress/mvt_expected
index e7d280fea0fc2ff4f6a1d0da980dcae83ea9eb49..75b6cb95d9b80f16ef929777e301ce35e8d23657 100644 (file)
--- a/regress/mvt_expected
+++ b/regress/mvt_expected
@@ -85,7 +85,6 @@ NDQxNkExQTNGOEQzQ0IwRjk0MEM3RjMzQ0E2RDQyRDU0NDE2QTFBM0Y4RDNDQjBGOTQwNDc0RDYy
 NzJBODE2NTQ0MUQ5NDBERkJBRkQ5RUYyNDA0NzRENjI3MkE4MTY1NDQxKIAgeAI=
 TA15|GkkKBHRlc3QSEBIGAAABAQIAGAEiBAky3j8aAmMxGgJjMhoHY3N0cmluZyIDCgExIhQKEjEyLjIz
 MjM4OTI4MzIyMzIzOSiAIHgC
-FI1|GjEKBHRlc3QSEAgBEgQAAAEBGAEiBAky3j8aAmMxGgJjMiICKAEiBgoEYWJjZCiAIHgC
 D1|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
 D2|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
 D3|Gi8KBHRlc3QSDhIEAAABARgBIgQJMt4/GgJjMRoCYzIiAigBIgYKBGFiY2QogCB4Ag==
@@ -97,3 +96,13 @@ TU2
 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==