From ce071a55c6697e31393703ae8a69e1445738c35c Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Fri, 13 Sep 2024 16:55:59 +0200
Subject: [PATCH] [OGR provider] Issue syncToDisk() after
 [add/delete/rename]Attributes()

Fixes #58669
---
 src/core/providers/ogr/qgsogrprovider.cpp | 27 +++++++++++++++++++
 tests/src/python/test_provider_ogr.py     | 33 +++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
index dafce60ba407..4c262e31d0fb 100644
--- a/src/core/providers/ogr/qgsogrprovider.cpp
+++ b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -2035,6 +2035,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;
 
@@ -2115,6 +2124,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 )
@@ -2169,6 +2187,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 )
diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py
index d90d4bd3c875..d76df78bdb82 100644
--- a/tests/src/python/test_provider_ogr.py
+++ b/tests/src/python/test_provider_ogr.py
@@ -3432,6 +3432,39 @@ def test_exporter_capabilities(self):
             self.assertFalse(
                 exporter.attributeEditCapabilities() & Qgis.VectorDataProviderAttributeEditCapability.EditComment)
 
+    @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()