diff --git a/autotest/ogr/ogr_gpkg.py b/autotest/ogr/ogr_gpkg.py index ad353c841165..92882c408c04 100755 --- a/autotest/ogr/ogr_gpkg.py +++ b/autotest/ogr/ogr_gpkg.py @@ -9178,8 +9178,10 @@ def test_ogr_gpkg_read_generated_column(tmp_vsimem): lyr = ds.GetLayer(0) assert lyr.GetLayerDefn().GetFieldCount() == 4 assert lyr.GetLayerDefn().GetFieldDefn(2).GetName() == "strfield_generated" + assert lyr.GetLayerDefn().GetFieldDefn(2).IsGenerated() assert lyr.GetLayerDefn().GetFieldDefn(2).GetType() == ogr.OFTString assert lyr.GetLayerDefn().GetFieldDefn(3).GetName() == "intfield_generated_stored" + assert lyr.GetLayerDefn().GetFieldDefn(3).IsGenerated() assert lyr.GetLayerDefn().GetFieldDefn(3).GetType() == ogr.OFTInteger64 f = ogr.Feature(lyr.GetLayerDefn()) @@ -9191,6 +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.GetFieldDefnRef(2).IsGenerated() + assert f.GetFieldDefnRef(3).IsGenerated() assert lyr.SetFeature(f) == ogr.OGRERR_NONE lyr.ResetReading() @@ -10914,8 +10918,56 @@ def test_ogr_gpkg_arrow_stream_numpy_datetime_as_string(tmp_vsimem): # Test field operations rolling back changes. -def test_ogr_gpkg_field_operations_rollback(tmp_vsimem): +@pytest.mark.parametrize("start_transaction", [False, True]) +def test_ogr_gpkg_field_operations_rollback(tmp_vsimem, start_transaction): filename = str(tmp_vsimem / "test.gpkg") with ogr.GetDriverByName("GPKG").CreateDataSource(filename) as ds: - ogrtest.check_transaction_rollback(ds, test_geometry=False) + ogrtest.check_transaction_rollback(ds, start_transaction, test_geometry=False) + + +@pytest.mark.parametrize("start_transaction", [False, True]) +def test_ogr_gpkg_field_operations_savepoint_rollback(tmp_vsimem, start_transaction): + + filename = str(tmp_vsimem / "test_savepoint.gpkg") + with ogr.GetDriverByName("GPKG").CreateDataSource(filename) as ds: + ogrtest.check_transaction_rollback_with_savepoint( + ds, start_transaction, test_geometry=False + ) + + +@pytest.mark.parametrize("auto_begin_transaction", [False, True]) +@pytest.mark.parametrize("start_transaction", [False, True]) +@pytest.mark.parametrize( + "release_to,rollback_to,expected", + ( + ([1], [], ["fld3"]), + ([2], [], ["fld3"]), + ([3], [], ["fld3"]), + ([4], [], ["fld3"]), + ([], [1], ["fld1", "fld2", "fld3", "fld4", "fld5"]), + ([], [2], ["fld1", "fld3", "fld4", "fld5"]), + ([], [3], ["fld1", "fld3", "fld5"]), + ([], [4], ["fld3", "fld5"]), + ), +) +def test_ogr_gpkg_field_operations_savepoint_release( + tmp_vsimem, + auto_begin_transaction, + start_transaction, + release_to, + rollback_to, + expected, +): + + filename = str(tmp_vsimem / "test_savepoint_release.gpkg") + ogrtest.check_transaction_savepoint_release( + filename, + "GPKG", + auto_begin_transaction, + start_transaction, + release_to, + rollback_to, + expected, + test_geometry=False, + ) diff --git a/autotest/ogr/ogr_sqlite.py b/autotest/ogr/ogr_sqlite.py index 898dd65168cd..c8248ac9d6f3 100755 --- a/autotest/ogr/ogr_sqlite.py +++ b/autotest/ogr/ogr_sqlite.py @@ -4543,10 +4543,56 @@ def test_ogr_sqlite_schema_override( # -def test_field_rollback(tmp_path): +@pytest.mark.parametrize("start_transaction", [False, True]) +def test_ogr_sqlite_field_operations_rollback(tmp_vsimem, start_transaction): - filename = str(tmp_path / "test_field_rollback.db") + filename = str(tmp_vsimem / "test.db") + with ogr.GetDriverByName("SQLite").CreateDataSource(filename) as ds: + ogrtest.check_transaction_rollback(ds, start_transaction, test_geometry=False) + + +@pytest.mark.parametrize("start_transaction", [False, True]) +def test_ogr_sqlite_field_operations_savepoint_rollback(tmp_vsimem, start_transaction): - # Create a new database. + filename = str(tmp_vsimem / "test_savepoint.db") with ogr.GetDriverByName("SQLite").CreateDataSource(filename) as ds: - ogrtest.check_transaction_rollback(ds, test_geometry=True) + ogrtest.check_transaction_rollback_with_savepoint( + ds, start_transaction, test_geometry=False + ) + + +@pytest.mark.parametrize("auto_begin_transaction", [False, True]) +@pytest.mark.parametrize("start_transaction", [False, True]) +@pytest.mark.parametrize( + "release_to,rollback_to,expected", + ( + ([1], [], ["fld3"]), + ([2], [], ["fld3"]), + ([3], [], ["fld3"]), + ([4], [], ["fld3"]), + ([], [1], ["fld1", "fld2", "fld3", "fld4", "fld5"]), + ([], [2], ["fld1", "fld3", "fld4", "fld5"]), + ([], [3], ["fld1", "fld3", "fld5"]), + ([], [4], ["fld3", "fld5"]), + ), +) +def test_ogr_sqlite_field_operations_savepoint_release( + tmp_vsimem, + auto_begin_transaction, + start_transaction, + release_to, + rollback_to, + expected, +): + + filename = str(tmp_vsimem / "test_savepoint_release.db") + ogrtest.check_transaction_savepoint_release( + filename, + "SQLite", + auto_begin_transaction, + start_transaction, + release_to, + rollback_to, + expected, + test_geometry=False, + ) diff --git a/autotest/pymod/ogrtest.py b/autotest/pymod/ogrtest.py index 956764c01a37..29db43b97596 100755 --- a/autotest/pymod/ogrtest.py +++ b/autotest/pymod/ogrtest.py @@ -31,7 +31,7 @@ import gdaltest import pytest -from osgeo import ogr +from osgeo import gdal, ogr geos_flag = None sfcgal_flag = None @@ -411,7 +411,9 @@ def spatial_filter(lyr, *args): # Check transactions rollback, to be called with a freshly created datasource -def check_transaction_rollback(ds, test_geometry=False): +def check_transaction_rollback(ds, start_transaction, test_geometry=False): + + gdal.ErrorReset() lyr = ds.CreateLayer("test", options=["GEOMETRY_NAME=geom"]) lyr.CreateField(ogr.FieldDefn("fld1", ogr.OFTString)) @@ -439,27 +441,27 @@ def verify(lyr, fld1, fld2): assert fld2.GetName() == "fld2" # Test deleting a field - ds.StartTransaction() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") lyr.DeleteField(0) # Test do not crash assert fld1.GetName() == "fld1" assert lyr.GetLayerDefn().GetFieldCount() == 1 assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" - ds.RollbackTransaction() + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") verify(lyr, fld1, fld2) # Test deleting the second field - ds.StartTransaction() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") lyr.DeleteField(1) assert lyr.GetLayerDefn().GetFieldCount() == 1 assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld1" - ds.RollbackTransaction() + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") verify(lyr, fld1, fld2) # Test renaming and changing the type of a field fld1 = lyr.GetLayerDefn().GetFieldDefn(0) assert fld1.GetName() == "fld1" - ds.StartTransaction() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") assert ( lyr.AlterFieldDefn( 0, ogr.FieldDefn("fld1_renamed", ogr.OFTInteger), ogr.ALTER_ALL_FLAG @@ -469,24 +471,24 @@ def verify(lyr, fld1, fld2): assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld1_renamed" assert lyr.GetLayerDefn().GetFieldDefn(0).GetType() == ogr.OFTInteger assert fld1.GetName() == "fld1_renamed" - ds.RollbackTransaction() + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") verify(lyr, fld1, fld2) # Test adding a field assert lyr.GetLayerDefn().GetFieldCount() == 2 - ds.StartTransaction() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") fld = ogr.FieldDefn("fld3", ogr.OFTInteger) assert lyr.CreateField(fld) == ogr.OGRERR_NONE assert lyr.GetLayerDefn().GetFieldCount() == 3 fld3 = lyr.GetLayerDefn().GetFieldDefn(2) assert fld3.GetName() == "fld3" - ds.RollbackTransaction() + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") verify(lyr, fld1, fld2) # Test fld3 does not crash assert fld3.GetName() == "fld3" # Test multiple operations - ds.StartTransaction() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") lyr.DeleteField(0) assert lyr.GetLayerDefn().GetFieldCount() == 1 assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" @@ -506,14 +508,17 @@ def verify(lyr, fld1, fld2): assert lyr.GetLayerDefn().GetFieldDefn(0).GetType() == ogr.OFTInteger assert lyr.GetLayerDefn().GetFieldDefn(1).GetName() == "fld3" assert lyr.GetLayerDefn().GetFieldDefn(1).GetType() == ogr.OFTInteger - ds.RollbackTransaction() + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") verify(lyr, fld1, fld2) if not test_geometry: return + ########################################################### + # Test geometry columns + # Start a transaction and add a geometry column. - assert ds.StartTransaction() == ogr.OGRERR_NONE + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") assert ( lyr.CreateGeomField(ogr.GeomFieldDefn("GEOMETRY_2", ogr.wkbPoint)) == ogr.OGRERR_NONE @@ -536,10 +541,258 @@ def verify(lyr, fld1, fld2): assert feat.GetField("fld2") == "value2-2" assert feat.GetGeomFieldRef(0).ExportToWkt() == "POINT (1 2)" assert feat.GetGeomFieldRef(1).ExportToWkt() == "POINT (3 4)" - assert ds.RollbackTransaction() == ogr.OGRERR_NONE + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") # Verify that we have not added GEOMETRY_2 field. assert lyr.GetGeometryColumn() == "geom" assert lyr.GetLayerDefn().GetGeomFieldCount() == 1 verify(lyr, fld1, fld2) + + +############################################################################### +# Check transactions rollback with savepoints, to be called with a freshly created datasource + + +def check_transaction_rollback_with_savepoint( + ds, start_transaction, test_geometry=False +): + + gdal.ErrorReset() + + assert gdal.GetLastErrorMsg() == "", gdal.GetLastErrorMsg() + lyr = ds.CreateLayer("test_rollback_with_savepoint", options=["GEOMETRY_NAME=geom"]) + lyr.CreateField(ogr.FieldDefn("fld1", ogr.OFTString)) + lyr.CreateField(ogr.FieldDefn("fld2", ogr.OFTString)) + + # Insert a feature + f = ogr.Feature(lyr.GetLayerDefn()) + f.SetField("fld1", "value1") + f.SetField("fld2", "value2") + assert lyr.CreateFeature(f) == ogr.OGRERR_NONE + + fld1 = lyr.GetLayerDefn().GetFieldDefn(0) + fld2 = lyr.GetLayerDefn().GetFieldDefn(1) + + def verify(lyr, fld1, fld2): + assert lyr.GetGeometryColumn() == "geom" + assert lyr.GetLayerDefn().GetGeomFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldCount() == 2 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld1" + assert lyr.GetLayerDefn().GetFieldDefn(0).GetType() == ogr.OFTString + assert lyr.GetLayerDefn().GetFieldDefn(1).GetName() == "fld2" + assert lyr.GetLayerDefn().GetFieldDefn(1).GetType() == ogr.OFTString + # Test do not crash + assert fld1.GetName() == "fld1" + assert fld2.GetName() == "fld2" + assert gdal.GetLastErrorMsg() == "", gdal.GetLastErrorMsg() + + # Test SAVEPOINT + ds.ExecuteSQL("SAVEPOINT test_savepoint1") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("ROLLBACK test_savepoint1") + + verify(lyr, fld1, fld2) + # Rollback TO leaves the transaction open, close it + ds.ExecuteSQL("ROLLBACK") + + # Test nested savepoints + ds.ExecuteSQL("SAVEPOINT test_savepoint2") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("SAVEPOINT test_savepoint3") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 0 + ds.ExecuteSQL("ROLLBACK test_savepoint3") + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("ROLLBACK test_savepoint2") + + verify(lyr, fld1, fld2) + # Rollback TO leaves the transaction open, close it + ds.ExecuteSQL("ROLLBACK") + + # Test rollback to first savepoint + ds.ExecuteSQL("SAVEPOINT test_savepoint5") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("SAVEPOINT test_savepoint6") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 0 + ds.ExecuteSQL("ROLLBACK test_savepoint5") + + verify(lyr, fld1, fld2) + # Rollback TO leaves the transaction open, close it + ds.ExecuteSQL("ROLLBACK") + + # Test rollback without specifying a savepoint + ds.ExecuteSQL("SAVEPOINT test_savepoint7") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("SAVEPOINT test_savepoint8") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 0 + ds.ExecuteSQL("ROLLBACK") + + verify(lyr, fld1, fld2) + + ########################################################### + # Test error conditions + assert gdal.GetLastErrorMsg() == "" + + # Test rollback to non-existing savepoint + ds.ExecuteSQL("SAVEPOINT test_savepoint9") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + + ds.ExecuteSQL("ROLLBACK test_savepointXXX") + assert gdal.GetLastErrorMsg().startswith("Savepoint test_savepointXXX not found") + gdal.ErrorReset() + ds.ExecuteSQL("ROLLBACK test_savepoint9") + + verify(lyr, fld1, fld2) + # Rollback TO leaves the transaction open, close it + ds.ExecuteSQL("ROLLBACK") + gdal.ErrorReset() + + # Test savepoint from within a transaction + assert gdal.GetLastErrorMsg() == "", gdal.GetLastErrorMsg() + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + assert lyr.GetLayerDefn().GetFieldDefn(0).GetName() == "fld2" + ds.ExecuteSQL("SAVEPOINT test_savepoint10") + assert gdal.GetLastErrorMsg() == "", gdal.GetLastErrorMsg() + # Rollback TO leaves the transaction open, close it + ds.RollbackTransaction() if start_transaction else ds.ExecuteSQL("ROLLBACK") + verify(lyr, fld1, fld2) + + +############################################################################### +# Check transactions rollback with savepoints release + + +def check_transaction_savepoint_release( + filename, + driver, + auto_begin_transaction, + start_transaction, + release_to, + rollback_to, + expected, + test_geometry=False, +): + + gdal.ErrorReset() + + def get_field_names(lyr): + field_names = [] + for i in range(lyr.GetLayerDefn().GetFieldCount()): + field_names.append(lyr.GetLayerDefn().GetFieldDefn(i).GetName()) + return field_names + + def verify(lyr): + + field_names = get_field_names(lyr) + + assert lyr.GetGeometryColumn() == "geom" + assert lyr.GetLayerDefn().GetGeomFieldCount() == 1 + assert field_names == expected, (field_names, expected) + + assert fld1 is not None + assert fld2 is not None + assert fld3 is not None + assert fld4 is not None + assert fld5 is not None + + with ogr.GetDriverByName(driver).CreateDataSource(filename) as ds: + + layer_name = "svp_release_%s" % ("_".join(str(x) for x in release_to)) + lyr = ds.CreateLayer(layer_name, options=["GEOMETRY_NAME=geom"]) + for i in range(1, 6): + lyr.CreateField(ogr.FieldDefn("fld%d" % i, ogr.OFTString)) + + # Insert a feature + f = ogr.Feature(lyr.GetLayerDefn()) + for i in range(1, 6): + f.SetField("fld%d" % i, "value%d" % i) + + assert lyr.CreateFeature(f) == ogr.OGRERR_NONE + + fld1 = lyr.GetLayerDefn().GetFieldDefn(0) + fld2 = lyr.GetLayerDefn().GetFieldDefn(1) + fld3 = lyr.GetLayerDefn().GetFieldDefn(2) + fld4 = lyr.GetLayerDefn().GetFieldDefn(3) + fld5 = lyr.GetLayerDefn().GetFieldDefn(4) + + assert fld1.GetName() == "fld1" + assert fld2.GetName() == "fld2" + assert fld3.GetName() == "fld3" + assert fld4.GetName() == "fld4" + assert fld5.GetName() == "fld5" + + assert lyr.GetLayerDefn().GetFieldCount() == 5 + + # fields are now 1,2,3,4,5 + assert get_field_names(lyr) == ["fld1", "fld2", "fld3", "fld4", "fld5"] + + # Test SAVEPOINT + if auto_begin_transaction: + ds.StartTransaction() if start_transaction else ds.ExecuteSQL("BEGIN") + + ds.ExecuteSQL("SAVEPOINT test_savepoint1") + assert lyr.DeleteField(1) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 4 + # fields are now 1,3,4,5 + assert get_field_names(lyr) == ["fld1", "fld3", "fld4", "fld5"] + + ds.ExecuteSQL("SAVEPOINT test_savepoint2") + assert lyr.DeleteField(2) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 3 + # fields are now 1,3,5 + assert get_field_names(lyr) == ["fld1", "fld3", "fld5"] + + ds.ExecuteSQL("SAVEPOINT test_savepoint3") + assert lyr.DeleteField(0) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 2 + # fields are now 3,5 + assert get_field_names(lyr) == ["fld3", "fld5"] + + ds.ExecuteSQL("SAVEPOINT test_savepoint4") + assert lyr.DeleteField(1) == ogr.OGRERR_NONE + assert lyr.GetLayerDefn().GetFieldCount() == 1 + # fields are now 3 + assert get_field_names(lyr) == ["fld3"] + + for i in release_to: + ds.ExecuteSQL("RELEASE test_savepoint%d" % i) + + for i in rollback_to: + ds.ExecuteSQL("ROLLBACK TO test_savepoint%d" % i) + + # Check that all savepoints have been released + for i in release_to: + assert ds.ExecuteSQL("ROLLBACK test_savepoint%d" % i) != ogr.OGRERR_NONE + assert gdal.GetLastErrorMsg().startswith( + "Savepoint test_savepoint%d not found" % i + ) + gdal.ErrorReset() + + # If the release is not to the last savepoint + # issue a COMMIT to close the transaction + if auto_begin_transaction or 1 not in release_to: + ds.CommitTransaction() if start_transaction else ds.ExecuteSQL("COMMIT") + + # Assert no errors + assert gdal.GetLastErrorMsg() == "", gdal.GetLastErrorMsg() + + # Reload the datasource and verify the state + with ogr.Open(filename) as ds: + lyr = ds.GetLayerByName(layer_name) + verify(lyr) diff --git a/ogr/ogr_api.h b/ogr/ogr_api.h index 1a854dc2c6f9..1b4563562603 100644 --- a/ogr/ogr_api.h +++ b/ogr/ogr_api.h @@ -380,6 +380,8 @@ int CPL_DLL OGR_Fld_IsIgnored(OGRFieldDefnH hDefn); void CPL_DLL OGR_Fld_SetIgnored(OGRFieldDefnH hDefn, int); int CPL_DLL OGR_Fld_IsNullable(OGRFieldDefnH hDefn); void CPL_DLL OGR_Fld_SetNullable(OGRFieldDefnH hDefn, int); +void CPL_DLL OGR_Fld_SetGenerated(OGRFieldDefnH hDefn, int); +int CPL_DLL OGR_Fld_IsGenerated(OGRFieldDefnH hDefn); int CPL_DLL OGR_Fld_IsUnique(OGRFieldDefnH hDefn); void CPL_DLL OGR_Fld_SetUnique(OGRFieldDefnH hDefn, int); const char CPL_DLL *OGR_Fld_GetDefault(OGRFieldDefnH hDefn); diff --git a/ogr/ogr_feature.h b/ogr/ogr_feature.h index b6e1bd181ee9..e223d81b49f4 100644 --- a/ogr/ogr_feature.h +++ b/ogr/ogr_feature.h @@ -82,6 +82,9 @@ class CPL_DLL OGRFieldDefn int bNullable; int bUnique; + // Used by drivers (GPKG) to track generated fields + bool m_bGenerated = false; + std::string m_osDomainName{}; // field domain name. Might be empty std::string m_osComment{}; // field comment. Might be empty @@ -192,6 +195,29 @@ class CPL_DLL OGRFieldDefn return bUnique; } + /** + * @brief Return whether the 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. + * @return TRUE if the field is a generated field, FALSE otherwise. + * @since GDAL 3.11 + */ + bool IsGenerated() const + { + return m_bGenerated; + } + + /** + * @brief SetGenerated set the field generated status. + * @param bGeneratedIn TRUE if the field is a generated field, FALSE otherwise. + * @since GDAL 3.11 + */ + void SetGenerated(bool bGeneratedIn) + { + m_bGenerated = bGeneratedIn; + } + void SetUnique(int bUniqueIn); const std::string &GetDomainName() const diff --git a/ogr/ogrfielddefn.cpp b/ogr/ogrfielddefn.cpp index bb9c234fc580..ef21ce2f65e6 100644 --- a/ogr/ogrfielddefn.cpp +++ b/ogr/ogrfielddefn.cpp @@ -68,6 +68,7 @@ OGRFieldDefn::OGRFieldDefn(const OGRFieldDefn *poPrototype) bIgnore(FALSE), // TODO(schwehr): Can we use IsIgnored()? eSubType(poPrototype->GetSubType()), bNullable(poPrototype->IsNullable()), bUnique(poPrototype->IsUnique()), + m_bGenerated(poPrototype->IsGenerated()), m_osDomainName(poPrototype->m_osDomainName), m_osComment(poPrototype->GetComment()), m_nTZFlag(poPrototype->GetTZFlag()) @@ -125,8 +126,9 @@ OGRFieldDefn::OGRFieldDefn(const OGRFieldDefn &oOther) pszDefault(oOther.pszDefault ? CPLStrdup(oOther.pszDefault) : nullptr), bIgnore(oOther.bIgnore), eSubType(oOther.eSubType), bNullable(oOther.bNullable), bUnique(oOther.bUnique), - m_osDomainName(oOther.m_osDomainName), m_osComment(oOther.m_osComment), - m_nTZFlag(oOther.m_nTZFlag), m_bSealed(oOther.m_bSealed) + m_bGenerated(oOther.m_bGenerated), m_osDomainName(oOther.m_osDomainName), + m_osComment(oOther.m_osComment), m_nTZFlag(oOther.m_nTZFlag), + m_bSealed(oOther.m_bSealed) { } @@ -158,6 +160,7 @@ OGRFieldDefn &OGRFieldDefn::operator=(const OGRFieldDefn &oOther) eSubType = oOther.eSubType; bNullable = oOther.bNullable; bUnique = oOther.bUnique; + m_bGenerated = oOther.m_bGenerated; m_osDomainName = oOther.m_osDomainName; m_osComment = oOther.m_osComment; m_nTZFlag = oOther.m_nTZFlag; @@ -1775,6 +1778,55 @@ void OGR_Fld_SetNullable(OGRFieldDefnH hDefn, int bNullableIn) OGRFieldDefn::FromHandle(hDefn)->SetNullable(bNullableIn); } +/************************************************************************/ +/* OGR_Fld_SetGenerated() */ +/************************************************************************/ + +/** + * \brief Set whether this field is a generated field. + * + * By default, fields are not generated, so this method is generally called with + * TRUE to set a generated field. + * + * This method is the same as the C++ method OGRFieldDefn::SetGenerated(). + * + * Note that once a OGRFieldDefn has been added to a layer definition with + * OGRLayer::AddFieldDefn(), its setter methods should not be called on the + * object returned with OGRLayer::GetLayerDefn()->GetFieldDefn(). Instead, + * OGRLayer::AlterFieldDefn() should be called on a new instance of + * OGRFieldDefn, for drivers that support AlterFieldDefn(). + * + * @param hDefn handle to the field definition + * @param bGeneratedIn FALSE if the field is a generated field. + * @since GDAL 3.11 + */ + +void OGR_Fld_SetGenerated(OGRFieldDefnH hDefn, int bGeneratedIn) +{ + OGRFieldDefn::FromHandle(hDefn)->SetGenerated(bGeneratedIn); +} + +/************************************************************************/ +/* OGR_Fld_IsGenerated() */ +/************************************************************************/ + +/** + * \brief Return whether this field is a generated field. + * + * By default, fields are not generated. + * + * This method is the same as the C++ method OGRFieldDefn::IsGenerated(). + * + * @param hDefn handle to the field definition + * @return TRUE if the field is a generated field. + * @since GDAL 3.11 + */ + +int OGR_Fld_IsGenerated(OGRFieldDefnH hDefn) +{ + return OGRFieldDefn::FromHandle(hDefn)->IsGenerated(); +} + /************************************************************************/ /* IsUnique() */ /************************************************************************/ diff --git a/ogr/ogrsf_frmts/generic/ogrlayer.cpp b/ogr/ogrsf_frmts/generic/ogrlayer.cpp index 3870e7d358f5..231963c18160 100644 --- a/ogr/ogrsf_frmts/generic/ogrlayer.cpp +++ b/ogr/ogrsf_frmts/generic/ogrlayer.cpp @@ -1913,17 +1913,32 @@ void OGRLayer::PrepareStartTransaction() /* FinishRollbackTransaction() */ /************************************************************************/ -void OGRLayer::FinishRollbackTransaction() +void OGRLayer::FinishRollbackTransaction(const std::string &osSavepointName) { // Deleted fields can be safely removed from the storage after being restored. std::vector toBeRemoved; + bool bSavepointFound = false; + // Loop through all changed fields and reset them to their previous state. for (int i = static_cast(m_apoFieldDefnChanges.size()) - 1; i >= 0; i--) { auto &oFieldChange = m_apoFieldDefnChanges[i]; + + if (!osSavepointName.empty()) + { + if (oFieldChange.osSavepointName == osSavepointName) + { + bSavepointFound = true; + } + else if (bSavepointFound) + { + continue; + } + } + CPLAssert(oFieldChange.poFieldDefn); const char *pszName = oFieldChange.poFieldDefn->GetNameRef(); const int iField = oFieldChange.iField; @@ -2011,11 +2026,29 @@ void OGRLayer::FinishRollbackTransaction() m_apoFieldDefnChanges.erase(m_apoFieldDefnChanges.begin() + i); } + /**********************************************************************/ + /* Reset geometry fields to their previous state. */ + /**********************************************************************/ + + bSavepointFound = false; + // Loop through all changed geometry fields and reset them to their previous state. for (int i = static_cast(m_apoGeomFieldDefnChanges.size()) - 1; i >= 0; i--) { auto &oGeomFieldChange = m_apoGeomFieldDefnChanges[i]; + + if (!osSavepointName.empty()) + { + if (oGeomFieldChange.osSavepointName == osSavepointName) + { + bSavepointFound = true; + } + else if (bSavepointFound) + { + continue; + } + } const char *pszName = oGeomFieldChange.poFieldDefn->GetNameRef(); const int iGeomField = oGeomFieldChange.iField; if (iGeomField >= 0) diff --git a/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h b/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h index 9c0c08797951..aa86550dda18 100644 --- a/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h +++ b/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h @@ -362,7 +362,7 @@ class GDALGeoPackageDataset final : public OGRSQLiteBaseDataSource, inline bool IsInTransaction() const { - return nSoftTransactionLevel > 0; + return m_nSoftTransactionLevel > 0 || m_aosSavepoints.size() > 0; } static std::string LaunderName(const std::string &osStr); @@ -678,8 +678,6 @@ class OGRGeoPackageTableLayer final : public OGRGeoPackageLayer bool m_bFeatureCountTriggersDeletedInTransaction = false; #endif CPLString m_soColumns{}; - std::vector m_abGeneratedColumns{}; // .size() == - // m_poFeatureDefn->GetFieldDefnCount() CPLString m_soFilter{}; CPLString osQuery{}; CPLString m_osRTreeName{}; diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp index f9ab53c0f1cc..304455e05361 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp @@ -7703,27 +7703,15 @@ OGRLayer *GDALGeoPackageDataset::ExecuteSQL(const char *pszSQLCommand, CSLDestroy(papszTokens); } - if (EQUAL(osSQLCommand, "VACUUM")) - { - ResetReadingAllLayers(); - } - - if (EQUAL(osSQLCommand, "BEGIN")) + if (ProcessTransactionSQL(osSQLCommand)) { - SoftStartTransaction(); return nullptr; } - else if (EQUAL(osSQLCommand, "COMMIT")) - { - SoftCommitTransaction(); - return nullptr; - } - else if (EQUAL(osSQLCommand, "ROLLBACK")) + + if (EQUAL(osSQLCommand, "VACUUM")) { - SoftRollbackTransaction(); - return nullptr; + ResetReadingAllLayers(); } - else if (STARTS_WITH_CI(osSQLCommand, "DELETE FROM ")) { // Optimize truncation of a table, especially if it has a spatial @@ -7742,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, "") && @@ -9678,7 +9665,7 @@ GDALGeoPackageDataset::GetLayerWithGetSpatialWhereByName(const char *pszName) OGRErr GDALGeoPackageDataset::CommitTransaction() { - if (nSoftTransactionLevel == 1) + if (m_nSoftTransactionLevel == 1) { FlushMetadata(); for (int i = 0; i < m_nLayers; i++) @@ -9701,7 +9688,7 @@ OGRErr GDALGeoPackageDataset::RollbackTransaction() std::vector abAddTriggers; std::vector abTriggersDeletedInTransaction; #endif - if (nSoftTransactionLevel == 1) + if (m_nSoftTransactionLevel == 1) { FlushMetadata(); for (int i = 0; i < m_nLayers; i++) diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp index 1407058c3b79..05349c3ed1b6 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp @@ -284,8 +284,8 @@ OGRErr OGRGeoPackageTableLayer::FeatureBindParameters( const int iField = nUpdatedFieldsCount < 0 ? idx : panUpdatedFieldsIdx[idx]; assert(iField >= 0); - if (iField == m_iFIDAsRegularColumnIndex || - m_abGeneratedColumns[iField]) + const auto &oFieldDefn = poFeatureDefn->GetFieldDefn(iField); + if (iField == m_iFIDAsRegularColumnIndex || oFieldDefn->IsGenerated()) continue; if (!poFeature->IsFieldSetUnsafe(iField)) { @@ -647,7 +647,8 @@ CPLString OGRGeoPackageTableLayer::FeatureGenerateInsertSQL( /* Add attribute column names (except FID) to the SQL */ for (int i = 0; i < poFeatureDefn->GetFieldCount(); i++) { - if (i == m_iFIDAsRegularColumnIndex || m_abGeneratedColumns[i]) + const auto &oFieldDefn = poFeatureDefn->GetFieldDefn(i); + if (i == m_iFIDAsRegularColumnIndex || oFieldDefn->IsGenerated()) continue; if (!bBindUnsetFields && !poFeature->IsFieldSet(i)) continue; @@ -764,7 +765,8 @@ std::string OGRGeoPackageTableLayer::FeatureGenerateUpdateSQL( const int nFieldCount = poFeatureDefn->GetFieldCount(); for (int i = 0; i < nFieldCount; i++) { - if (i == m_iFIDAsRegularColumnIndex || m_abGeneratedColumns[i]) + const auto &oFieldDefn = poFeatureDefn->GetFieldDefn(i); + if (i == m_iFIDAsRegularColumnIndex || oFieldDefn->IsGenerated()) continue; if (!poFeature->IsFieldSet(i)) continue; @@ -1109,7 +1111,6 @@ OGRErr OGRGeoPackageTableLayer::ReadTableDefinition() m_pszTableName); } - m_abGeneratedColumns.resize(oResultTable->RowCount()); for (int iRecord = 0; iRecord < oResultTable->RowCount(); iRecord++) { const char *pszName = oResultTable->GetValue(1, iRecord); @@ -1317,15 +1318,12 @@ OGRErr OGRGeoPackageTableLayer::ReadTableDefinition() oField.SetDefault(pszDefault); } } - m_abGeneratedColumns[m_poFeatureDefn->GetFieldCount()] = - bIsGenerated; + oField.SetGenerated(bIsGenerated); m_poFeatureDefn->AddFieldDefn(&oField); } } } - m_abGeneratedColumns.resize(m_poFeatureDefn->GetFieldCount()); - /* Wait, we didn't find a FID? Some operations will not be possible */ if (m_bIsTable && m_pszFidColumn == nullptr) { @@ -1850,12 +1848,9 @@ OGRErr OGRGeoPackageTableLayer::CreateField(const OGRFieldDefn *poField, m_apoFieldDefnChanges.emplace_back( std::make_unique(oFieldDefn), m_poFeatureDefn->GetFieldCount() - 1, FieldChangeType::ADD_FIELD, - /* bGenerated */ false); + m_poDS->GetCurrentSavepoint()); } - m_abGeneratedColumns.resize(m_poFeatureDefn->GetFieldCount()); - m_abGeneratedColumns.back() = false; // explicit is better than implicit - if (m_pszFidColumn != nullptr && EQUAL(oFieldDefn.GetNameRef(), m_pszFidColumn)) { @@ -2031,8 +2026,7 @@ OGRGeoPackageTableLayer::CreateGeomField(const OGRGeomFieldDefn *poGeomFieldIn, { m_apoGeomFieldDefnChanges.emplace_back( std::make_unique(oGeomField), - m_poFeatureDefn->GetGeomFieldCount(), FieldChangeType::ADD_FIELD, - /* bGenerated */ false); + m_poFeatureDefn->GetGeomFieldCount(), FieldChangeType::ADD_FIELD); } whileUnsealing(m_poFeatureDefn)->AddGeomFieldDefn(&oGeomField); @@ -3261,7 +3255,7 @@ std::string OGRGeoPackageTableLayer::FeatureGenerateUpdateSQL( { const int iField = panUpdatedFieldsIdx[i]; if (iField == m_iFIDAsRegularColumnIndex || - m_abGeneratedColumns[iField]) + poFeatureDefn->GetFieldDefn(iField)->IsGenerated()) continue; if (!poFeature->IsFieldSet(iField)) continue; @@ -3815,42 +3809,6 @@ bool OGRGeoPackageTableLayer::DoJobAtTransactionRollback() m_bDeferredSpatialIndexCreation = bDeferredSpatialIndexCreationBackup; } - // If we are restoring any deleted field or removing any added one we have to - // rebuild the array of generated fields - for (int i = static_cast(m_apoFieldDefnChanges.size()) - 1; i >= 0; - i--) - { - auto &oFieldChange = m_apoFieldDefnChanges[i]; - switch (oFieldChange.eChangeType) - { - case FieldChangeType::ADD_FIELD: - { - CPLAssert(oFieldChange.iField < - static_cast(m_abGeneratedColumns.size())); - m_abGeneratedColumns.erase(m_abGeneratedColumns.begin() + - oFieldChange.iField); - break; - }; - case FieldChangeType::DELETE_FIELD: - { - CPLAssert(oFieldChange.iField <= - static_cast(m_abGeneratedColumns.size())); - m_abGeneratedColumns.insert(m_abGeneratedColumns.begin() + - oFieldChange.iField, - oFieldChange.bGenerated); - break; - }; - case FieldChangeType::ALTER_FIELD: - { - CPLAssert(oFieldChange.iField < - static_cast(m_abGeneratedColumns.size())); - m_abGeneratedColumns[oFieldChange.iField] = - oFieldChange.bGenerated; - break; - }; - } - } - ResetReading(); return true; } @@ -6496,7 +6454,8 @@ OGRErr OGRGeoPackageTableLayer::DeleteField(int iFieldToDelete) const GPKGTemporaryForeignKeyCheckDisabler oGPKGTemporaryForeignKeyCheckDisabler(m_poDS); - if (m_poDS->SoftStartTransaction() != OGRERR_NONE) + if (m_poDS->GetCurrentSavepoint().empty() && + m_poDS->SoftStartTransaction() != OGRERR_NONE) { return OGRERR_FAILURE; } @@ -6618,7 +6577,10 @@ OGRErr OGRGeoPackageTableLayer::DeleteField(int iFieldToDelete) /* -------------------------------------------------------------------- */ if (eErr == OGRERR_NONE) { - eErr = m_poDS->SoftCommitTransaction(); + + if (m_poDS->GetCurrentSavepoint().empty()) + eErr = m_poDS->SoftCommitTransaction(); + if (eErr == OGRERR_NONE) { @@ -6631,7 +6593,7 @@ OGRErr OGRGeoPackageTableLayer::DeleteField(int iFieldToDelete) m_apoFieldDefnChanges.emplace_back( std::move(poFieldDefn), iFieldToDelete, FieldChangeType::DELETE_FIELD, - m_abGeneratedColumns[iFieldToDelete]); + m_poDS->GetCurrentSavepoint()); } else { @@ -6643,26 +6605,10 @@ OGRErr OGRGeoPackageTableLayer::DeleteField(int iFieldToDelete) eErr = whileUnsealing(m_poFeatureDefn) ->DeleteFieldDefn(iFieldToDelete); } - - if (eErr == OGRERR_NONE) - { -#if SQLITE_VERSION_NUMBER >= 3035005L - CPLAssert(iFieldToDelete < - static_cast(m_abGeneratedColumns.size())); - m_abGeneratedColumns.erase(m_abGeneratedColumns.begin() + - iFieldToDelete); -#else - // We have recreated the table from scratch, and lost the - // generated column property - std::fill(m_abGeneratedColumns.begin(), - m_abGeneratedColumns.end(), false); -#endif - } - ResetReading(); } } - else + else if (m_poDS->GetCurrentSavepoint().empty()) { m_poDS->SoftRollbackTransaction(); } @@ -6789,7 +6735,7 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn(int iFieldToAlter, /* Build the modified field definition from the flags. */ /* -------------------------------------------------------------------- */ OGRFieldDefn oTmpFieldDefn(poFieldDefnToAlter); - bool bUseRewriteSchemaMethod = (m_poDS->nSoftTransactionLevel == 0); + bool bUseRewriteSchemaMethod = (m_poDS->m_nSoftTransactionLevel == 0); int nActualFlags = 0; if (bRenameCol) { @@ -7142,7 +7088,7 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn(int iFieldToAlter, m_apoFieldDefnChanges.emplace_back( std::make_unique(poFieldDefnToAlter), iFieldToAlter, FieldChangeType::ALTER_FIELD, - m_abGeneratedColumns[iFieldToAlter]); + m_poDS->GetCurrentSavepoint()); } auto oTemporaryUnsealer(poFieldDefnToAlter->GetTemporaryUnsealer()); @@ -7612,14 +7558,6 @@ OGRErr OGRGeoPackageTableLayer::ReorderFields(int *panMap) eErr = whileUnsealing(m_poFeatureDefn)->ReorderFieldDefns(panMap); } - if (eErr == OGRERR_NONE) - { - // We have recreated the table from scratch, and lost the - // generated column property - std::fill(m_abGeneratedColumns.begin(), m_abGeneratedColumns.end(), - false); - } - ResetReading(); } else diff --git a/ogr/ogrsf_frmts/ogrsf_frmts.h b/ogr/ogrsf_frmts/ogrsf_frmts.h index f0f75b150bb4..0d774075baa4 100644 --- a/ogr/ogrsf_frmts/ogrsf_frmts.h +++ b/ogr/ogrsf_frmts/ogrsf_frmts.h @@ -286,7 +286,8 @@ class CPL_DLL OGRLayer : public GDALMajorObject //! @cond Doxygen_Suppress // Keep field definitions in sync with transactions virtual void PrepareStartTransaction(); - virtual void FinishRollbackTransaction(); + // Rollback TO SAVEPOINT if osSavepointName is not empty, otherwise ROLLBACK + virtual void FinishRollbackTransaction(const std::string &osSavepointName); //! @endcond virtual const char *GetFIDColumn(); @@ -414,17 +415,17 @@ class CPL_DLL OGRLayer : public GDALMajorObject { FieldDefnChange(std::unique_ptr &&poFieldDefnIn, int iFieldIn, - FieldChangeType eChangeTypeIn, bool bGeneratedIn) + FieldChangeType eChangeTypeIn, + const std::string &osSavepointNameIn = "") : poFieldDefn(std::move(poFieldDefnIn)), iField(iFieldIn), - eChangeType(eChangeTypeIn), bGenerated(bGeneratedIn) + eChangeType(eChangeTypeIn), osSavepointName(osSavepointNameIn) { } std::unique_ptr poFieldDefn; int iField; FieldChangeType eChangeType; - // Used by drivers (GPKG) to track generated fields - bool bGenerated; + std::string osSavepointName; }; std::vector> m_apoFieldDefnChanges{}; diff --git a/ogr/ogrsf_frmts/pg/ogr_pg.h b/ogr/ogrsf_frmts/pg/ogr_pg.h index 190cf2c27d08..2e756164f5e9 100644 --- a/ogr/ogrsf_frmts/pg/ogr_pg.h +++ b/ogr/ogrsf_frmts/pg/ogr_pg.h @@ -344,8 +344,6 @@ class OGRPGTableLayer final : public OGRPGLayer CPLString m_osFirstGeometryFieldName{}; - std::vector m_abGeneratedColumns{}; - std::string m_osLCOGeomType{}; virtual CPLString GetFromClauseForGetExtent() override diff --git a/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp index d202d7700bd8..f98e9134069f 100644 --- a/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp +++ b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp @@ -827,10 +827,10 @@ int OGRPGTableLayer::ReadTableDefinition() if (pszDescription) oField.SetComment(pszDescription); + oField.SetGenerated(pszGenerated != nullptr && pszGenerated[0] != '\0'); + // CPLDebug("PG", "name=%s, type=%s", oField.GetNameRef(), pszType); poFeatureDefn->AddFieldDefn(&oField); - m_abGeneratedColumns.push_back(pszGenerated != nullptr && - pszGenerated[0] != '\0'); } OGRPGClearResult(hResult); @@ -1574,7 +1574,7 @@ OGRErr OGRPGTableLayer::IUpdateFeature(OGRFeature *poFeature, continue; if (!poFeature->IsFieldSet(iField)) continue; - if (m_abGeneratedColumns[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 (m_abGeneratedColumns[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 (m_abGeneratedColumns[i]) + if (poFeature->GetDefnRef()->GetFieldDefn(i)->IsGenerated()) continue; if (bNeedComma) @@ -2204,9 +2204,11 @@ OGRErr OGRPGTableLayer::CreateFeatureViaCopy(OGRFeature *poFeature) } } - std::vector abFieldsToInclude(m_abGeneratedColumns.size(), true); + std::vector abFieldsToInclude(poFeature->GetFieldCount(), true); for (size_t i = 0; i < abFieldsToInclude.size(); i++) - abFieldsToInclude[i] = !m_abGeneratedColumns[i]; + abFieldsToInclude[i] = !poFeature->GetDefnRef() + ->GetFieldDefn(static_cast(i)) + ->IsGenerated(); if (bFIDColumnInCopyFields) { @@ -2481,7 +2483,6 @@ OGRErr OGRPGTableLayer::CreateField(const OGRFieldDefn *poFieldIn, } whileUnsealing(poFeatureDefn)->AddFieldDefn(&oField); - m_abGeneratedColumns.resize(poFeatureDefn->GetFieldCount()); if (pszFIDColumn != nullptr && EQUAL(oField.GetNameRef(), pszFIDColumn)) { @@ -2739,8 +2740,6 @@ OGRErr OGRPGTableLayer::DeleteField(int iField) OGRPGClearResult(hResult); - m_abGeneratedColumns.erase(m_abGeneratedColumns.begin() + iField); - return whileUnsealing(poFeatureDefn)->DeleteFieldDefn(iField); } @@ -3584,7 +3583,8 @@ CPLString OGRPGTableLayer::BuildCopyFields() { if (i == nFIDIndex) continue; - if (m_abGeneratedColumns[i]) + + if (poFeatureDefn->GetFieldDefn(i)->IsGenerated()) continue; const char *pszName = poFeatureDefn->GetFieldDefn(i)->GetNameRef(); diff --git a/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h b/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h index 8f70475b7c63..d4621fbe7994 100644 --- a/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h +++ b/ogr/ogrsf_frmts/sqlite/ogr_sqlite.h @@ -777,7 +777,6 @@ class OGRSQLiteDataSource final : public OGRSQLiteBaseDataSource virtual OGRErr StartTransaction(int bForce = FALSE) override; virtual OGRErr CommitTransaction() override; virtual OGRErr RollbackTransaction() override; - bool IsInTransaction() const; virtual char **GetMetadata(const char *pszDomain = "") override; diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h index c06babada8f0..bf9642721456 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h @@ -143,8 +143,11 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset bool InitSpatialite(); void FinishSpatialite(); - int bUserTransactionActive = FALSE; - int nSoftTransactionLevel = 0; + int m_bUserTransactionActive = FALSE; + int m_nSoftTransactionLevel = 0; + std::vector m_aosSavepoints{}; + // The transaction was implicitly started by SAVEPOINT + bool m_bImplicitTransactionOpened = false; OGRErr DoTransactionCommand(const char *pszCommand); @@ -156,6 +159,18 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset OGRSQLiteBaseDataSource(); virtual ~OGRSQLiteBaseDataSource(); + std::string GetCurrentSavepoint() const + { + return m_aosSavepoints.empty() ? "" : m_aosSavepoints.back(); + } + + std::string GetFirstSavepoint() const + { + return m_aosSavepoints.empty() ? "" : m_aosSavepoints.front(); + } + + bool IsInTransaction() const; + sqlite3 *GetDB() { return hDB; @@ -200,6 +215,15 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset OGRErr SoftStartTransaction(); OGRErr SoftCommitTransaction(); OGRErr SoftRollbackTransaction(); + OGRErr StartSavepoint(const std::string &osName); + 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); diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp index d51c2b0aa947..959d65baad92 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp @@ -3004,7 +3004,8 @@ bool OGRSQLiteDataSource::OpenVirtualTable(const char *pszName, { OGRGeometry *poGeom = poFeature->GetGeometryRef(); if (poGeom) - poLayer->GetLayerDefn()->SetGeomType(poGeom->getGeometryType()); + whileUnsealing(poLayer->GetLayerDefn()) + ->SetGeomType(poGeom->getGeometryType()); delete poFeature; } poLayer->ResetReading(); @@ -3443,9 +3444,11 @@ OGRLayer *OGRSQLiteDataSource::ExecuteSQL(const char *pszSQLCommand, } } } + else if (ProcessTransactionSQL(pszSQLCommand)) + { + return nullptr; + } else if (!STARTS_WITH_CI(pszSQLCommand, "SELECT ") && - !EQUAL(pszSQLCommand, "BEGIN") && - !EQUAL(pszSQLCommand, "COMMIT") && !STARTS_WITH_CI(pszSQLCommand, "CREATE TABLE ") && !STARTS_WITH_CI(pszSQLCommand, "PRAGMA ")) { @@ -4016,24 +4019,26 @@ OGRErr OGRSQLiteDataSource::DeleteLayer(int iLayer) OGRErr OGRSQLiteBaseDataSource::StartTransaction(CPL_UNUSED int bForce) { - if (bUserTransactionActive || nSoftTransactionLevel != 0) + if (m_bUserTransactionActive || m_nSoftTransactionLevel != 0) { CPLError(CE_Failure, CPLE_AppDefined, "Transaction already established"); return OGRERR_FAILURE; } - for (int i = 0; i < GetLayerCount(); i++) + // Check if we are in a SAVEPOINT transaction + if (m_aosSavepoints.size() > 0) { - OGRLayer *poLayer = GetLayer(i); - poLayer->PrepareStartTransaction(); + CPLError(CE_Failure, CPLE_AppDefined, + "Cannot start a transaction within a SAVEPOINT"); + return OGRERR_FAILURE; } OGRErr eErr = SoftStartTransaction(); if (eErr != OGRERR_NONE) return eErr; - bUserTransactionActive = true; + m_bUserTransactionActive = true; return OGRERR_NONE; } @@ -4060,21 +4065,22 @@ OGRErr OGRSQLiteDataSource::StartTransaction(int bForce) OGRErr OGRSQLiteBaseDataSource::CommitTransaction() { - if (!bUserTransactionActive) + if (!m_bUserTransactionActive && !m_bImplicitTransactionOpened) { CPLError(CE_Failure, CPLE_AppDefined, "Transaction not established"); return OGRERR_FAILURE; } - bUserTransactionActive = false; - CPLAssert(nSoftTransactionLevel == 1); + m_bUserTransactionActive = false; + m_bImplicitTransactionOpened = false; + CPLAssert(m_nSoftTransactionLevel == 1); return SoftCommitTransaction(); } OGRErr OGRSQLiteDataSource::CommitTransaction() { - if (nSoftTransactionLevel == 1) + if (m_nSoftTransactionLevel == 1) { for (auto &poLayer : m_apoLayers) { @@ -4098,21 +4104,14 @@ OGRErr OGRSQLiteDataSource::CommitTransaction() OGRErr OGRSQLiteBaseDataSource::RollbackTransaction() { - if (!bUserTransactionActive) + if (!m_bUserTransactionActive) { CPLError(CE_Failure, CPLE_AppDefined, "Transaction not established"); return OGRERR_FAILURE; } - bUserTransactionActive = false; - CPLAssert(nSoftTransactionLevel == 1); - - // Loop through all layers and finish transaction - for (int i = 0; i < GetLayerCount(); i++) - { - OGRLayer *poLayer = GetLayer(i); - poLayer->FinishRollbackTransaction(); - } + m_bUserTransactionActive = false; + CPLAssert(m_nSoftTransactionLevel == 1); return SoftRollbackTransaction(); } @@ -4120,7 +4119,7 @@ OGRErr OGRSQLiteBaseDataSource::RollbackTransaction() OGRErr OGRSQLiteDataSource::RollbackTransaction() { - if (nSoftTransactionLevel == 1) + if (m_nSoftTransactionLevel == 1) { for (auto &poLayer : m_apoLayers) { @@ -4142,9 +4141,9 @@ OGRErr OGRSQLiteDataSource::RollbackTransaction() return OGRSQLiteBaseDataSource::RollbackTransaction(); } -bool OGRSQLiteDataSource::IsInTransaction() const +bool OGRSQLiteBaseDataSource::IsInTransaction() const { - return nSoftTransactionLevel > 0; + return m_nSoftTransactionLevel > 0; } /************************************************************************/ @@ -4158,11 +4157,17 @@ bool OGRSQLiteDataSource::IsInTransaction() const OGRErr OGRSQLiteBaseDataSource::SoftStartTransaction() { - nSoftTransactionLevel++; + m_nSoftTransactionLevel++; OGRErr eErr = OGRERR_NONE; - if (nSoftTransactionLevel == 1) + if (m_nSoftTransactionLevel == 1) { + for (int i = 0; i < GetLayerCount(); i++) + { + OGRLayer *poLayer = GetLayer(i); + poLayer->PrepareStartTransaction(); + } + eErr = DoTransactionCommand("BEGIN"); } @@ -4185,15 +4190,15 @@ OGRErr OGRSQLiteBaseDataSource::SoftCommitTransaction() // CPLDebug("SQLite", "%p->SoftCommitTransaction() : %d", // this, nSoftTransactionLevel); - if (nSoftTransactionLevel <= 0) + if (m_nSoftTransactionLevel <= 0) { CPLAssert(false); return OGRERR_FAILURE; } OGRErr eErr = OGRERR_NONE; - nSoftTransactionLevel--; - if (nSoftTransactionLevel == 0) + m_nSoftTransactionLevel--; + if (m_nSoftTransactionLevel == 0) { eErr = DoTransactionCommand("COMMIT"); } @@ -4214,22 +4219,218 @@ OGRErr OGRSQLiteBaseDataSource::SoftRollbackTransaction() // CPLDebug("SQLite", "%p->SoftRollbackTransaction() : %d", // this, nSoftTransactionLevel); - if (nSoftTransactionLevel <= 0) + while (!m_aosSavepoints.empty()) + { + if (RollbackToSavepoint(m_aosSavepoints.back()) != OGRERR_NONE) + { + return OGRERR_FAILURE; + } + m_aosSavepoints.pop_back(); + } + + if (m_nSoftTransactionLevel <= 0) { CPLAssert(false); return OGRERR_FAILURE; } OGRErr eErr = OGRERR_NONE; - nSoftTransactionLevel--; - if (nSoftTransactionLevel == 0) + m_nSoftTransactionLevel--; + if (m_nSoftTransactionLevel == 0) { eErr = DoTransactionCommand("ROLLBACK"); + if (eErr == OGRERR_NONE) + { + for (int i = 0; i < GetLayerCount(); i++) + { + OGRLayer *poLayer = GetLayer(i); + poLayer->FinishRollbackTransaction(""); + } + } } return eErr; } +OGRErr OGRSQLiteBaseDataSource::StartSavepoint(const std::string &osName) +{ + + // A SAVEPOINT implicity starts a transaction, let's fake one + if (!IsInTransaction()) + { + m_bImplicitTransactionOpened = true; + m_nSoftTransactionLevel++; + for (int i = 0; i < GetLayerCount(); i++) + { + OGRLayer *poLayer = GetLayer(i); + poLayer->PrepareStartTransaction(); + } + } + + const std::string osCommand = "SAVEPOINT " + osName; + const auto eErr = DoTransactionCommand(osCommand.c_str()); + + if (eErr == OGRERR_NONE) + { + m_aosSavepoints.push_back(osName); + } + + return eErr; +} + +OGRErr OGRSQLiteBaseDataSource::ReleaseSavepoint(const std::string &osName) +{ + if (m_aosSavepoints.empty() || + std::find(m_aosSavepoints.cbegin(), m_aosSavepoints.cend(), osName) == + m_aosSavepoints.cend()) + { + CPLError(CE_Failure, CPLE_AppDefined, "Savepoint %s not found", + osName.c_str()); + return OGRERR_FAILURE; + } + + const std::string osCommand = "RELEASE SAVEPOINT " + osName; + const auto eErr = DoTransactionCommand(osCommand.c_str()); + + if (eErr == OGRERR_NONE) + { + // If the savepoint is the outer most, this is the same as COMMIT + // and the transaction is closed + if (m_bImplicitTransactionOpened && + m_aosSavepoints.front().compare(osName) == 0) + { + m_bImplicitTransactionOpened = false; + m_bUserTransactionActive = false; + m_nSoftTransactionLevel = 0; + m_aosSavepoints.clear(); + } + else + { + // Find all savepoints up to the target one and remove them + while (!m_aosSavepoints.empty() && m_aosSavepoints.back() != osName) + { + m_aosSavepoints.pop_back(); + } + m_aosSavepoints.pop_back(); + } + } + return eErr; +} + +OGRErr OGRSQLiteBaseDataSource::RollbackToSavepoint(const std::string &osName) +{ + if (m_aosSavepoints.empty() || + std::find(m_aosSavepoints.cbegin(), m_aosSavepoints.cend(), osName) == + m_aosSavepoints.cend()) + { + CPLError(CE_Failure, CPLE_AppDefined, "Savepoint %s not found", + osName.c_str()); + return OGRERR_FAILURE; + } + + const std::string osCommand = "ROLLBACK TO SAVEPOINT " + osName; + const auto eErr = DoTransactionCommand(osCommand.c_str()); + + if (eErr == OGRERR_NONE) + { + + // The target savepoint should become the last one in the list + // and does not need to be removed because ROLLBACK TO SAVEPOINT + while (!m_aosSavepoints.empty() && m_aosSavepoints.back() != osName) + { + m_aosSavepoints.pop_back(); + } + } + + for (int i = 0; i < GetLayerCount(); i++) + { + OGRLayer *poLayer = GetLayer(i); + poLayer->FinishRollbackTransaction(osName); + } + + return eErr; +} + +/************************************************************************/ +/* ProcessTransactionSQL() */ +/************************************************************************/ +bool OGRSQLiteBaseDataSource::ProcessTransactionSQL( + const std::string &osSQLCommand) +{ + bool retVal = true; + + if (EQUAL(osSQLCommand.c_str(), "BEGIN")) + { + SoftStartTransaction(); + } + else if (EQUAL(osSQLCommand.c_str(), "COMMIT")) + { + SoftCommitTransaction(); + } + else if (EQUAL(osSQLCommand.c_str(), "ROLLBACK")) + { + SoftRollbackTransaction(); + } + else if (STARTS_WITH_CI(osSQLCommand.c_str(), "SAVEPOINT")) + { + const CPLStringList aosTokens(SQLTokenize(osSQLCommand.c_str())); + if (aosTokens.size() == 2) + { + const char *pszSavepointName = aosTokens[1]; + StartSavepoint(pszSavepointName); + } + else + { + retVal = false; + } + } + else if (STARTS_WITH_CI(osSQLCommand.c_str(), "RELEASE")) + { + const CPLStringList aosTokens(SQLTokenize(osSQLCommand.c_str())); + if (aosTokens.size() == 2) + { + const char *pszSavepointName = aosTokens[1]; + ReleaseSavepoint(pszSavepointName); + } + else if (aosTokens.size() == 3 && EQUAL(aosTokens[1], "SAVEPOINT")) + { + const char *pszSavepointName = aosTokens[2]; + ReleaseSavepoint(pszSavepointName); + } + else + { + retVal = false; + } + } + else if (STARTS_WITH_CI(osSQLCommand.c_str(), "ROLLBACK")) + { + const CPLStringList aosTokens(SQLTokenize(osSQLCommand.c_str())); + if (aosTokens.size() == 2) + { + if (EQUAL(aosTokens[1], "TRANSACTION")) + { + SoftRollbackTransaction(); + } + else + { + const char *pszSavepointName = aosTokens[1]; + RollbackToSavepoint(pszSavepointName); + } + } + else if (aosTokens.size() > 1) // Savepoint name is last token + { + const char *pszSavepointName = aosTokens[aosTokens.size() - 1]; + RollbackToSavepoint(pszSavepointName); + } + } + else + { + retVal = false; + } + + return retVal; +} + /************************************************************************/ /* DoTransactionCommand() */ /************************************************************************/ diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp index 0579138e371d..bc4dd42ae3e4 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp @@ -225,6 +225,7 @@ void OGRSQLiteTableLayer::SetCreationParameters(const char *pszFIDColumnName, poGeomFieldDefn->SetSpatialRef(poSRS); m_poFeatureDefn->AddGeomFieldDefn(std::move(poGeomFieldDefn)); } + m_poFeatureDefn->Seal(/* bSealFields */ true); } /************************************************************************/ @@ -852,6 +853,8 @@ OGRFeatureDefn *OGRSQLiteTableLayer::GetLayerDefn() else LoadStatistics(); + m_poFeatureDefn->Seal(/* bSealFields */ true); + return m_poFeatureDefn; } @@ -1611,14 +1614,14 @@ OGRErr OGRSQLiteTableLayer::CreateField(const OGRFieldDefn *poFieldIn, /* -------------------------------------------------------------------- */ /* Add the field to the OGRFeatureDefn. */ /* -------------------------------------------------------------------- */ - m_poFeatureDefn->AddFieldDefn(&oField); + whileUnsealing(m_poFeatureDefn)->AddFieldDefn(&oField); if (m_poDS->IsInTransaction()) { m_apoFieldDefnChanges.emplace_back( std::make_unique(oField), m_poFeatureDefn->GetFieldCount() - 1, FieldChangeType::ADD_FIELD, - /* bGenerated */ false); + m_poDS->GetCurrentSavepoint()); } if (m_pszFIDColumn != nullptr && EQUAL(oField.GetNameRef(), m_pszFIDColumn)) @@ -1724,8 +1727,7 @@ OGRSQLiteTableLayer::CreateGeomField(const OGRGeomFieldDefn *poGeomFieldIn, { m_apoGeomFieldDefnChanges.emplace_back( std::make_unique(*poGeomField), - m_poFeatureDefn->GetGeomFieldCount(), FieldChangeType::ADD_FIELD, - /* bGenerated */ false); + m_poFeatureDefn->GetGeomFieldCount(), FieldChangeType::ADD_FIELD); } m_poFeatureDefn->AddGeomFieldDefn(std::move(poGeomField)); @@ -2174,12 +2176,14 @@ OGRErr OGRSQLiteTableLayer::DeleteField(int iFieldToDelete) if (m_poDS->IsInTransaction()) { std::unique_ptr poFieldDefn; - poFieldDefn = m_poFeatureDefn->StealFieldDefn(iFieldToDelete); + poFieldDefn = whileUnsealing(m_poFeatureDefn) + ->StealFieldDefn(iFieldToDelete); if (poFieldDefn) { m_apoFieldDefnChanges.emplace_back( std::move(poFieldDefn), iFieldToDelete, - FieldChangeType::DELETE_FIELD, false); + FieldChangeType::DELETE_FIELD, + m_poDS->GetCurrentSavepoint()); } else { @@ -2188,7 +2192,8 @@ OGRErr OGRSQLiteTableLayer::DeleteField(int iFieldToDelete) } else { - eErr = m_poFeatureDefn->DeleteFieldDefn(iFieldToDelete); + eErr = whileUnsealing(m_poFeatureDefn) + ->DeleteFieldDefn(iFieldToDelete); } RecomputeOrdinals(); @@ -2410,13 +2415,14 @@ OGRErr OGRSQLiteTableLayer::AlterFieldDefn(int iFieldToAlter, /* Finish */ /* -------------------------------------------------------------------- */ + auto oTemporaryUnsealer(m_poFeatureDefn->GetTemporaryUnsealer()); OGRFieldDefn *poFieldDefn = m_poFeatureDefn->GetFieldDefn(iFieldToAlter); if (m_poDS->IsInTransaction()) { m_apoFieldDefnChanges.emplace_back( std::make_unique(poFieldDefn), iFieldToAlter, - FieldChangeType::ALTER_FIELD, /* bGenerated */ false); + FieldChangeType::ALTER_FIELD, m_poDS->GetCurrentSavepoint()); } if (nActualFlags & ALTER_TYPE_FLAG) @@ -2587,7 +2593,7 @@ OGRErr OGRSQLiteTableLayer::ReorderFields(int *panMap) /* Finish */ /* -------------------------------------------------------------------- */ - eErr = m_poFeatureDefn->ReorderFieldDefns(panMap); + eErr = whileUnsealing(m_poFeatureDefn)->ReorderFieldDefns(panMap); RecomputeOrdinals(); diff --git a/swig/include/ogr.i b/swig/include/ogr.i index 5054c3128d37..55c06ae1dd11 100644 --- a/swig/include/ogr.i +++ b/swig/include/ogr.i @@ -2977,6 +2977,14 @@ public: OGR_Fld_SetUnique( self, bUnique ); } + int IsGenerated() { + return OGR_Fld_IsGenerated( self ); + } + + void SetGenerated(int bGenerated ) { + OGR_Fld_SetGenerated( self, bGenerated ); + } + const char* GetDefault() { return OGR_Fld_GetDefault( self ); }