ST_DumpPoints is not null safe

[robe]

SELECT ST_DumpPoints(foo1.the_geom)
	FROM ((SELECT CAST(Null As geometry) As the_geom)) As foo1;

Gives error -

ERROR:  Unexpected error while dumping geometry <NULL>
CONTEXT:  SQL function "st_dumppoints" statement 1

[yabo]

Here's a patch. I'm not sure of what should be in the regress test though. Empty lines ? NULL ? I can't run make check from where I am now.

Index: postgis/postgis.sql.in.c
===================================================================
--- postgis/postgis.sql.in.c    (revision 6203)
+++ postgis/postgis.sql.in.c    (working copy)
@@ -1950,6 +1950,10 @@
 
   -- Special case collections : iterate and return the DumpPoints of the geometries
 
+  IF (the_geom IS NULL OR ST_IsEmpty(the_geom)) THEN
+    RETURN;
+  END IF;
+
   IF (ST_IsCollection(the_geom)) THEN
  
     i = 1;
Index: regress/dumppoints.sql
===================================================================
--- regress/dumppoints.sql      (revision 6203)
+++ regress/dumppoints.sql      (working copy)
@@ -157,3 +157,19 @@
         )'::geometry AS geom
     ) AS g
   ) j;
+
+SELECT path, ST_AsText(geom)
+FROM (
+  SELECT (ST_DumpPoints(g.geom)).*
+  FROM
+    (SELECT NULL::geometry AS geom
+    ) AS g
+  ) j;
+
+SELECT path, ST_AsText(geom)
+FROM (
+  SELECT (ST_DumpPoints(g.geom)).*
+  FROM
+    (SELECT 'GEOMETRYCOLLECTION EMPTY'::geometry AS geom
+    ) AS g
+  ) j;
Index: regress/dumppoints_expected
===================================================================
--- regress/dumppoints_expected (revision 6203)
+++ regress/dumppoints_expected (working copy)
@@ -82,3 +82,5 @@
 {5,1,2,2}|POINT(5 6)
 {5,1,2,3}|POINT(6 6)
 {5,1,2,4}|POINT(5 5)
+
+

[robe]

I was wondering if it wouldn't be sufficient to just mark this function as STRICT like the way ST_Dump is marked. Do people think its necessary to even put logic about NULL handling in the function if the function is marked as strict.

Even though this is an SQL function, I don't see any benefit of keeping it non-strict since it could never utilize spatial indexes anyway.

[robe]

I think this one is easy to knock off too -- opinions?

[pramsey]

Don't ask, do.

[robe]

Fixed at r7360 for 1.5 and r7361 for PostGIS 2.0.0. Note the _ST_DumpPoints still has an issue thought no one would be calling that directly. Can't make that strict since it should have nulls passed in. For a more permanent solution - we might want to just go with yabo's suggestion.

With my fix, dumping a NULL will return no records which is hmm okay with me.