Factor out SQL transaction and remove C api

OSGeo · Jan 20, 2025 · a64554c · a64554c
1 parent 3356983
commit a64554c
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 205 deletions.
diff --git a/autotest/ogr/ogr_gpkg.py b/autotest/ogr/ogr_gpkg.py
@@ -9193,8 +9193,8 @@ def test_ogr_gpkg_read_generated_column(tmp_vsimem):
     assert f["strfield"] == "foo"
     assert f["strfield_generated"] == "foo_generated"
     assert f["intfield_generated_stored"] == 5
-    assert f.IsFieldGenerated(2)
-    assert f.IsFieldGenerated(3)
+    assert f.GetFieldDefnRef(2).IsGenerated()
+    assert f.GetFieldDefnRef(3).IsGenerated()
 
     assert lyr.SetFeature(f) == ogr.OGRERR_NONE
     lyr.ResetReading()

diff --git a/ogr/ogr_api.h b/ogr/ogr_api.h
@@ -480,7 +480,6 @@ OGRFieldDefnH CPL_DLL OGR_F_GetFieldDefnRef(OGRFeatureH, int);
 int CPL_DLL OGR_F_GetFieldIndex(OGRFeatureH, const char *);
 
 int CPL_DLL OGR_F_IsFieldSet(OGRFeatureH, int);
-int CPL_DLL OGR_F_IsFieldGenerated(OGRFeatureH, int);
 void CPL_DLL OGR_F_UnsetField(OGRFeatureH, int);
 
 int CPL_DLL OGR_F_IsFieldNull(OGRFeatureH, int);

diff --git a/ogr/ogr_feature.h b/ogr/ogr_feature.h
@@ -1270,8 +1270,6 @@ class CPL_DLL OGRFeature
 
     int IsFieldSet(int iField) const;
 
-    bool IsFieldGenerated(int iField) const;
-
     void UnsetField(int iField);
 
     bool IsFieldNull(int iField) const;

diff --git a/ogr/ogrfeature.cpp b/ogr/ogrfeature.cpp
@@ -1590,66 +1590,6 @@ int OGRFeature::IsFieldSet(int iField) const
     }
 }
 
-/************************************************************************/
-/*                             IsFieldGenerated()                       */
-/************************************************************************/
-
-/**
- * \brief Test if a field is a generated field.
- *
- * At time of writing, only the GeoPackage and PG drivers fill that information. Consequently,
- * only a returned value equal to TRUE can be fully trusted.
- *
- * This method is the same as the C function OGR_F_IsFieldGenerated().
- *
- * @param iField the field to test.
- *
- * @return TRUE if the field has been generated, otherwise false.
- *
- * @since GDAL 3.11
- */
-
-bool OGRFeature::IsFieldGenerated(int iField) const
-
-{
-    const auto oFieldDefn = poDefn->GetFieldDefn(iField);
-    return oFieldDefn ? oFieldDefn->IsGenerated() : false;
-}
-
-/************************************************************************/
-/*                          OGR_F_IsFieldGenerated()                    */
-/************************************************************************/
-
-/**
- * \brief Test if a field is a generated field.
- *
- * At time of writing, only the GeoPackage driver fills that information. Consequently,
- * only a returned value equal to TRUE can be fully trusted.
- * This function is the same as the C++ method OGRFeature::IsFieldGenerated().
- *
- * @param hFeat handle to the feature on which the field is.
- * @param iField the field to test.
- *
- * @return TRUE if the field has been generated, otherwise false.
- *
- * @since GDAL 3.11
- */
-
-int OGR_F_IsFieldGenerated(OGRFeatureH hFeat, int iField)
-
-{
-    VALIDATE_POINTER1(hFeat, "OGR_F_IsFieldGenerated", 0);
-
-    const OGRFeature *poFeature = OGRFeature::FromHandle(hFeat);
-    if (iField < 0 || iField >= poFeature->GetFieldCount())
-    {
-        CPLError(CE_Failure, CPLE_AppDefined, "Invalid index : %d", iField);
-        return FALSE;
-    }
-
-    return poFeature->IsFieldGenerated(iField);
-}
-
 /************************************************************************/
 /*                          OGR_F_IsFieldSet()                          */
 /************************************************************************/

diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp
@@ -7703,75 +7703,14 @@ OGRLayer *GDALGeoPackageDataset::ExecuteSQL(const char *pszSQLCommand,
         CSLDestroy(papszTokens);
     }
 
-    if (EQUAL(osSQLCommand, "VACUUM"))
-    {
-        ResetReadingAllLayers();
-    }
-
-    if (EQUAL(osSQLCommand, "BEGIN"))
-    {
-        SoftStartTransaction();
-        return nullptr;
-    }
-    else if (EQUAL(osSQLCommand, "COMMIT"))
-    {
-        SoftCommitTransaction();
-        return nullptr;
-    }
-    else if (EQUAL(osSQLCommand, "ROLLBACK"))
+    if (ProcessTransactionSQL(osSQLCommand))
     {
-        SoftRollbackTransaction();
         return nullptr;
     }
-    else if (STARTS_WITH_CI(osSQLCommand, "SAVEPOINT"))
-    {
-        const CPLStringList aosTokens(SQLTokenize(osSQLCommand));
-        if (aosTokens.size() == 2)
-        {
-            const char *pszSavepointName = aosTokens[1];
-            StartSavepoint(pszSavepointName);
-            return nullptr;
-        }
-    }
-    else if (STARTS_WITH_CI(osSQLCommand, "RELEASE"))
-    {
-        const CPLStringList aosTokens(SQLTokenize(osSQLCommand));
-        if (aosTokens.size() == 2)
-        {
-            const char *pszSavepointName = aosTokens[1];
-            ReleaseSavepoint(pszSavepointName);
-            return nullptr;
-        }
-        else if (aosTokens.size() == 3 && EQUAL(aosTokens[1], "SAVEPOINT"))
-        {
-            const char *pszSavepointName = aosTokens[2];
-            ReleaseSavepoint(pszSavepointName);
-            return nullptr;
-        }
-    }
-    else if (STARTS_WITH_CI(osSQLCommand, "ROLLBACK"))
+
+    if (EQUAL(osSQLCommand, "VACUUM"))
     {
-        const CPLStringList aosTokens(SQLTokenize(osSQLCommand));
-        if (aosTokens.size() == 2)
-        {
-            if (EQUAL(aosTokens[1], "TRANSACTION"))
-            {
-                SoftRollbackTransaction();
-                return nullptr;
-            }
-            else
-            {
-                const char *pszSavepointName = aosTokens[1];
-                RollbackToSavepoint(pszSavepointName);
-                return nullptr;
-            }
-        }
-        else if (aosTokens.size() > 1)  // Savepoint name is last token
-        {
-            const char *pszSavepointName = aosTokens[aosTokens.size() - 1];
-            RollbackToSavepoint(pszSavepointName);
-            return nullptr;
-        }
+        ResetReadingAllLayers();
     }
     else if (STARTS_WITH_CI(osSQLCommand, "DELETE FROM "))
     {
@@ -7791,7 +7730,6 @@ OGRLayer *GDALGeoPackageDataset::ExecuteSQL(const char *pszSQLCommand,
             }
         }
     }
-
     else if (pszDialect != nullptr && EQUAL(pszDialect, "INDIRECT_SQLITE"))
         return GDALDataset::ExecuteSQL(osSQLCommand, poSpatialFilter, "SQLITE");
     else if (pszDialect != nullptr && !EQUAL(pszDialect, "") &&

diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
@@ -3255,7 +3255,7 @@ std::string OGRGeoPackageTableLayer::FeatureGenerateUpdateSQL(
     {
         const int iField = panUpdatedFieldsIdx[i];
         if (iField == m_iFIDAsRegularColumnIndex ||
-            poFeature->IsFieldGenerated(iField))
+            poFeatureDefn->GetFieldDefn(iField)->IsGenerated())
             continue;
         if (!poFeature->IsFieldSet(iField))
             continue;

diff --git a/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp
@@ -1574,7 +1574,7 @@ OGRErr OGRPGTableLayer::IUpdateFeature(OGRFeature *poFeature,
             continue;
         if (!poFeature->IsFieldSet(iField))
             continue;
-        if (poFeature->IsFieldGenerated(iField))
+        if (poFeature->GetDefnRef()->GetFieldDefn(iField)->IsGenerated())
             continue;
 
         if (bNeedComma)
@@ -1934,7 +1934,7 @@ OGRErr OGRPGTableLayer::CreateFeatureViaInsert(OGRFeature *poFeature)
             continue;
         if (!poFeature->IsFieldSet(i))
             continue;
-        if (poFeature->IsFieldGenerated(i))
+        if (poFeature->GetDefnRef()->GetFieldDefn(i)->IsGenerated())
             continue;
 
         if (!bNeedComma)
@@ -2058,7 +2058,7 @@ OGRErr OGRPGTableLayer::CreateFeatureViaInsert(OGRFeature *poFeature)
             continue;
         if (!poFeature->IsFieldSet(i))
             continue;
-        if (poFeature->IsFieldGenerated(i))
+        if (poFeature->GetDefnRef()->GetFieldDefn(i)->IsGenerated())
             continue;
 
         if (bNeedComma)
@@ -2206,8 +2206,9 @@ OGRErr OGRPGTableLayer::CreateFeatureViaCopy(OGRFeature *poFeature)
 
     std::vector<bool> abFieldsToInclude(poFeature->GetFieldCount(), true);
     for (size_t i = 0; i < abFieldsToInclude.size(); i++)
-        abFieldsToInclude[i] =
-            !poFeature->IsFieldGenerated(static_cast<int>(i));
+        abFieldsToInclude[i] = !poFeature->GetDefnRef()
+                                    ->GetFieldDefn(static_cast<int>(i))
+                                    ->IsGenerated();
 
     if (bFIDColumnInCopyFields)
     {

diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h
@@ -219,6 +219,12 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset
     OGRErr ReleaseSavepoint(const std::string &osName);
     OGRErr RollbackToSavepoint(const std::string &osName);
 
+    /**
+     *  Execute a SQL transaction command (BEGIN, COMMIT, ROLLBACK, SAVEPOINT)
+     *  @return TRUE if the osSQLCommand was recognized as a transaction command
+     */
+    bool ProcessTransactionSQL(const std::string &osSQLCommand);
+
     OGRErr PragmaCheck(const char *pszPragma, const char *pszExpected,
                        int nRowsExpected);