Merge pull request #9437 from rouault/fix_duckdb_spatial_273

GeoJSON reader: accept a {"type": "Polygon","coordinates": []} as a representation of POLYGON EMPTY (fixes duckdb/duckdb_spatial#273)
OSGeo · Mar 15, 2024 · 2daa551 · 2daa551
2 parents 35feb24 + c7e2ca8
commit 2daa551
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 54 deletions.
diff --git a/autotest/ogr/ogr_geojson.py b/autotest/ogr/ogr_geojson.py
@@ -1906,7 +1906,7 @@ def test_ogr_geojson_50():
 
 
 ###############################################################################
-# Test writing empty geometries
+# Test writing and reading empty geometries
 
 
 def test_ogr_geojson_51():
@@ -1949,8 +1949,6 @@ def test_ogr_geojson_51():
     data = gdal.VSIFReadL(1, 10000, fp).decode("ascii")
     gdal.VSIFCloseL(fp)
 
-    gdal.Unlink("/vsimem/ogr_geojson_51.json")
-
     assert '{ "id": 1 }, "geometry": null' in data
 
     assert (
@@ -1982,6 +1980,14 @@ def test_ogr_geojson_51():
         in data
     )
 
+    ds = ogr.Open("/vsimem/ogr_geojson_51.json")
+    lyr = ds.GetLayer(0)
+    for f in lyr:
+        if f.GetFID() >= 2:
+            assert f.GetGeometryRef().IsEmpty()
+
+    gdal.Unlink("/vsimem/ogr_geojson_51.json")
+
 
 ###############################################################################
 # Test NULL type detection

diff --git a/frmts/netcdf/netcdfsgwriterutil.cpp b/frmts/netcdf/netcdfsgwriterutil.cpp
@@ -37,7 +37,7 @@ SGeometry_Feature::SGeometry_Feature(OGRFeature &ft)
 
     if (geom == nullptr)
     {
-        throw SGWriter_Exception_EmptyGeometry();
+        throw SGWriter_Exception_NullGeometry();
     }
 
     OGRwkbGeometryType ogwkt = geom->getGeometryType();
@@ -114,12 +114,8 @@ SGeometry_Feature::SGeometry_Feature(OGRFeature &ft)
         // Get node count
         // First count exterior ring
         const auto exterior_ring = poly->getExteriorRing();
-        if (exterior_ring == nullptr)
-        {
-            throw SGWriter_Exception_EmptyGeometry();
-        }
-
-        size_t outer_ring_ct = exterior_ring->getNumPoints();
+        const size_t outer_ring_ct =
+            exterior_ring ? exterior_ring->getNumPoints() : 0;
 
         this->total_point_count += outer_ring_ct;
         this->ppart_node_count.push_back(outer_ring_ct);
@@ -134,14 +130,12 @@ SGeometry_Feature::SGeometry_Feature(OGRFeature &ft)
         {
             this->hasInteriorRing = true;
             const auto iring = poly->getInteriorRing(iRingCt);
-            if (iring == nullptr)
+            if (iring)
             {
-                throw SGWriter_Exception_RingOOB();
+                this->total_point_count += iring->getNumPoints();
+                this->ppart_node_count.push_back(iring->getNumPoints());
+                this->total_part_count++;
             }
-
-            this->total_point_count += iring->getNumPoints();
-            this->ppart_node_count.push_back(iring->getNumPoints());
-            this->total_part_count++;
         }
     }
 
@@ -155,12 +149,8 @@ SGeometry_Feature::SGeometry_Feature(OGRFeature &ft)
         for (const auto poly : *poMP)
         {
             const auto exterior_ring = poly->getExteriorRing();
-            if (exterior_ring == nullptr)
-            {
-                throw SGWriter_Exception_EmptyGeometry();
-            }
-
-            size_t outer_ring_ct = exterior_ring->getNumPoints();
+            const size_t outer_ring_ct =
+                exterior_ring ? exterior_ring->getNumPoints() : 0;
 
             this->total_point_count += outer_ring_ct;
             this->ppart_node_count.push_back(outer_ring_ct);
@@ -176,16 +166,14 @@ SGeometry_Feature::SGeometry_Feature(OGRFeature &ft)
                  iRingCt++)
             {
                 const auto iring = poly->getInteriorRing(iRingCt);
-                if (iring == nullptr)
+                if (iring)
                 {
-                    throw SGWriter_Exception_RingOOB();
+                    this->hasInteriorRing = true;
+                    this->total_point_count += iring->getNumPoints();
+                    this->ppart_node_count.push_back(iring->getNumPoints());
+                    this->total_part_count++;
+                    this->part_at_ind_interior.push_back(true);
                 }
-
-                this->hasInteriorRing = true;
-                this->total_point_count += iring->getNumPoints();
-                this->ppart_node_count.push_back(iring->getNumPoints());
-                this->total_part_count++;
-                this->part_at_ind_interior.push_back(true);
             }
         }
     }

diff --git a/frmts/netcdf/netcdfsgwriterutil.h b/frmts/netcdf/netcdfsgwriterutil.h
@@ -639,7 +639,7 @@ class SGWriter_Exception_NCDefFailure : public SGWriter_Exception
                                     const char *failure_type);
 };
 
-class SGWriter_Exception_EmptyGeometry : public SGWriter_Exception
+class SGWriter_Exception_NullGeometry : public SGWriter_Exception
 {
     std::string msg;
 
@@ -648,28 +648,13 @@ class SGWriter_Exception_EmptyGeometry : public SGWriter_Exception
     {
         return this->msg.c_str();
     }
-    SGWriter_Exception_EmptyGeometry()
-        : msg("An empty geometry was detected when writing a netCDF file. "
+    SGWriter_Exception_NullGeometry()
+        : msg("A null  geometry was detected when writing a netCDF file. "
               "Empty geometries are not allowed.")
     {
     }
 };
 
-class SGWriter_Exception_RingOOB : public SGWriter_Exception
-{
-    std::string msg;
-
-  public:
-    const char *get_err_msg() override
-    {
-        return this->msg.c_str();
-    }
-    SGWriter_Exception_RingOOB()
-        : msg("An attempt was made to read a polygon ring that does not exist.")
-    {
-    }
-};
-
 class SGWriter_Exception_NCDelFailure : public SGWriter_Exception
 {
     std::string msg;

diff --git a/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp b/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp
@@ -3003,7 +3003,6 @@ OGRPolygon *OGRGeoJSONReadPolygon(json_object *poObj, bool bRaw)
             if (poObjPoints == nullptr)
             {
                 poPolygon = new OGRPolygon();
-                poPolygon->addRingDirectly(new OGRLinearRing());
             }
             else
             {
@@ -3019,11 +3018,7 @@ OGRPolygon *OGRGeoJSONReadPolygon(json_object *poObj, bool bRaw)
                  i < nRings && nullptr != poPolygon; ++i)
             {
                 poObjPoints = json_object_array_get_idx(poObjRings, i);
-                if (poObjPoints == nullptr)
-                {
-                    poPolygon->addRingDirectly(new OGRLinearRing());
-                }
-                else
+                if (poObjPoints != nullptr)
                 {
                     OGRLinearRing *poRing =
                         OGRGeoJSONReadLinearRing(poObjPoints);
@@ -3034,6 +3029,10 @@ OGRPolygon *OGRGeoJSONReadPolygon(json_object *poObj, bool bRaw)
                 }
             }
         }
+        else
+        {
+            poPolygon = new OGRPolygon();
+        }
     }
 
     return poPolygon;