Skip to content

Commit

Permalink
[OGR provider] Issue syncToDisk() after [add/delete/rename]Attributes()
Browse files Browse the repository at this point in the history
Fixes #58669
  • Loading branch information
rouault authored and nyalldawson committed Sep 13, 2024
1 parent 14b7567 commit 995a1cb
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 0 deletions.
27 changes: 27 additions & 0 deletions src/core/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2211,6 +2211,15 @@ bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )
}
}

// We at least want to syncToDisc() for OpenFileGDB, because its AddField
// implementation doesn't update immediately system tables.
// We exclude GeoJSON because leaveUpdateMode() has specific behavior for it.
if ( mGDALDriverName != QLatin1String( "GeoJSON" ) && !syncToDisc() )

{
returnvalue = false;
}

// Backup existing fields. We need them to 'restore' field type, length, precision
QgsFields oldFields = mAttributeFields;

Expand Down Expand Up @@ -2291,6 +2300,15 @@ bool QgsOgrProvider::deleteAttributes( const QgsAttributeIds &attributes )
res = false;
}
}

// We at least want to syncToDisc() for OpenFileGDB, because its DeleteField
// implementation doesn't update immediately system tables.
// We exclude GeoJSON because leaveUpdateMode() has specific behavior for it.
if ( mGDALDriverName != QLatin1String( "GeoJSON" ) && !syncToDisc() )
{
res = false;
}

loadFields();

if ( mTransaction )
Expand Down Expand Up @@ -2345,6 +2363,15 @@ bool QgsOgrProvider::renameAttributes( const QgsFieldNameMap &renamedAttributes
result = false;
}
}

// We at least want to syncToDisc() for OpenFileGDB, because its AlterFieldDefn
// implementation doesn't update immediately system tables.
// We exclude GeoJSON because leaveUpdateMode() has specific behavior for it.
if ( mGDALDriverName != QLatin1String( "GeoJSON" ) && !syncToDisc() )
{
result = false;
}

loadFields();

if ( mTransaction )
Expand Down
34 changes: 34 additions & 0 deletions tests/src/python/test_provider_ogr.py
Original file line number Diff line number Diff line change
Expand Up @@ -3615,6 +3615,7 @@ def testExtentShp(self):
del vl

@unittest.skipIf(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(3, 6, 0), "GDAL 3.6 required")
@unittest.skipIf(gdal.GetDriverByName("OpenFileGDB") is None, "GDAL OpenFileGDB driver required")
def testReadOnlyFieldsFileGeodatabase(self):

with tempfile.TemporaryDirectory() as temp_dir:
Expand All @@ -3626,6 +3627,39 @@ def testReadOnlyFieldsFileGeodatabase(self):
vl = QgsVectorLayer(dest_file_name, 'vl')
self.assertTrue(vl.fields()["Shape_Area"].isReadOnly())

@unittest.skipIf(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(3, 6, 0), "GDAL 3.6 required")
@unittest.skipIf(gdal.GetDriverByName("OpenFileGDB") is None, "GDAL OpenFileGDB driver required")
def testDeleteFieldFileGeodatabase(self):

with tempfile.TemporaryDirectory() as temp_dir:
dest_file_name = os.path.join(temp_dir, 'testDeleteFieldFileGeodatabase.gdb')
ds = ogr.GetDriverByName("OpenFileGDB").CreateDataSource(dest_file_name)
lyr = ds.CreateLayer("test", geom_type=ogr.wkbNone)
lyr.CreateField(ogr.FieldDefn("fld1"))
lyr.CreateField(ogr.FieldDefn("fld2"))
f = ogr.Feature(lyr.GetLayerDefn())
f["fld1"] = "a"
f["fld2"] = "b"
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f["fld1"] = "c"
f["fld2"] = "d"
lyr.CreateFeature(f)
ds = None

vl = QgsVectorLayer(dest_file_name, 'vl')
self.assertTrue(vl.startEditing())
self.assertTrue(vl.deleteAttribute(1)) # delete field fld1
self.assertTrue(vl.commitChanges())

# Re-open a connection without explicitly closing the one we've edited
vl2 = QgsVectorLayer(dest_file_name, 'vl2')
features = {f.id(): f.attributes() for f in vl2.getFeatures()}
self.assertEqual(features, {
1: [1, 'b'],
2: [2, 'd']
})


if __name__ == '__main__':
unittest.main()

0 comments on commit 995a1cb

Please sign in to comment.