ExecuteSQL() SQLITE dialect: error out if SetSpatialFilter() fails (f…

…ixes OSGeo#9623)
rouault · Apr 5, 2024 · f407b87 · f407b87
1 parent 846349b
commit f407b87
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 11 deletions.
diff --git a/autotest/ogr/ogr_sql_sqlite.py b/autotest/ogr/ogr_sql_sqlite.py
@@ -2477,3 +2477,36 @@ def test_ogr_sql_sqlite_like_utf8():
         dialect="SQLite",
     ) as sql_lyr:
         assert sql_lyr.GetFeatureCount() == 0
+
+
+###############################################################################
+# Test error on setting a spatial filter during ExecuteSQL
+
+
+@gdaltest.enable_exceptions()
+def test_ogr_sql_sqlite_execute_sql_error_on_spatial_filter_mem_layer():
+
+    ds = ogr.GetDriverByName("Memory").CreateDataSource("")
+    ds.CreateLayer("test")
+    geom = ogr.CreateGeometryFromWkt("POLYGON((0 0,0 1,1 1,1 0,0 0))")
+    with pytest.raises(
+        Exception, match="Cannot set spatial filter: no geometry field selected"
+    ):
+        ds.ExecuteSQL("SELECT 1 FROM test", spatialFilter=geom, dialect="SQLITE")
+
+
+###############################################################################
+# Test error on setting a spatial filter during ExecuteSQL
+
+
+@gdaltest.enable_exceptions()
+def test_ogr_sql_sqlite_execute_sql_error_on_spatial_filter_shp_layer(tmp_vsimem):
+
+    filename = str(tmp_vsimem / "test.shp")
+    ds = ogr.GetDriverByName("ESRI Shapefile").CreateDataSource(filename)
+    ds.CreateLayer("test")
+    geom = ogr.CreateGeometryFromWkt("POLYGON((0 0,0 1,1 1,1 0,0 0))")
+    with pytest.raises(
+        Exception, match="Cannot set spatial filter: no geometry field selected"
+    ):
+        ds.ExecuteSQL("SELECT 1 FROM test", spatialFilter=geom, dialect="SQLITE")
diff --git a/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h b/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h
@@ -1160,6 +1160,16 @@ class OGRGeoPackageSelectLayer final : public OGRGeoPackageLayer,
     {
         return OGRGeoPackageLayer::GetExtent(iGeomField, psExtent, bForce);
     }
+
+    bool
+    ValidateGeometryFieldIndexForSetSpatialFilter(int iGeomField,
+                                                  const OGRGeometry *poGeomIn,
+                                                  bool bIsSelectLayer) override
+    {
+        return OGRGeoPackageLayer::
+            ValidateGeometryFieldIndexForSetSpatialFilter(iGeomField, poGeomIn,
+                                                          bIsSelectLayer);
+    }
 };
 
 #endif /* OGR_GEOPACKAGE_H_INCLUDED */
diff --git a/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h b/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h
@@ -656,6 +656,15 @@ class OGRSQLiteSelectLayer CPL_NON_FINAL : public OGRSQLiteLayer,
     {
         return OGRSQLiteLayer::GetExtent(iGeomField, psExtent, bForce);
     }
+
+    bool
+    ValidateGeometryFieldIndexForSetSpatialFilter(int iGeomField,
+                                                  const OGRGeometry *poGeomIn,
+                                                  bool bIsSelectLayer) override
+    {
+        return OGRSQLiteLayer::ValidateGeometryFieldIndexForSetSpatialFilter(
+            iGeomField, poGeomIn, bIsSelectLayer);
+    }
 };
 
 /************************************************************************/

diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h
@@ -279,6 +279,8 @@ class IOGRSQLiteSelectLayer
     virtual OGRErr BaseGetExtent(OGREnvelope *psExtent, int bForce) = 0;
     virtual OGRErr BaseGetExtent(int iGeomField, OGREnvelope *psExtent,
                                  int bForce) = 0;
+    virtual bool ValidateGeometryFieldIndexForSetSpatialFilter(
+        int iGeomField, const OGRGeometry *poGeomIn, bool bIsSelectLayer) = 0;
 };
 
 /************************************************************************/

diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqliteexecutesql.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqliteexecutesql.cpp
@@ -1161,7 +1161,15 @@ OGRLayer *OGRSQLiteExecuteSQL(GDALDataset *poDS, const char *pszStatement,
         bUseStatementForGetNextFeature, bEmptyLayer, bCanReopenBaseDS);
 
     if (poSpatialFilter != nullptr)
+    {
+        const auto nErrorCounter = CPLGetErrorCounter();
         poLayer->SetSpatialFilter(0, poSpatialFilter);
+        if (CPLGetErrorCounter() > nErrorCounter)
+        {
+            delete poLayer;
+            return nullptr;
+        }
+    }
 
     if (poSingleSrcLayer != nullptr)
         poLayer->SetMetadata(poSingleSrcLayer->GetMetadata("NATIVE_DATA"),

diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp
@@ -382,6 +382,9 @@ void OGRSQLiteSelectLayer::SetSpatialFilter(int iGeomField,
 {
     if (!m_bCanReopenBaseDS && iGeomField == 0)
     {
+        if (!ValidateGeometryFieldIndexForSetSpatialFilter(iGeomField, poGeomIn,
+                                                           true))
+            return;
         // For a Memory datasource, short-circuit
         // OGRSQLiteExecuteSQL::SetSpatialFilter()
         // that would try to re-open the Memory datasource, which would fail.
@@ -397,18 +400,9 @@ void OGRSQLiteSelectLayerCommonBehaviour::SetSpatialFilter(
     int iGeomField, OGRGeometry *poGeomIn)
 
 {
-    if (iGeomField == 0 && poGeomIn == nullptr &&
-        m_poLayer->GetLayerDefn()->GetGeomFieldCount() == 0)
-    {
-        /* do nothing */
-    }
-    else if (iGeomField < 0 ||
-             iGeomField >= m_poLayer->GetLayerDefn()->GetGeomFieldCount())
-    {
-        CPLError(CE_Failure, CPLE_AppDefined,
-                 "Invalid geometry field index : %d", iGeomField);
+    if (!m_poLayer->ValidateGeometryFieldIndexForSetSpatialFilter(
+            iGeomField, poGeomIn, true))
         return;
-    }
 
     m_bAllowResetReadingEvenIfIndexAtZero = true;