OGR: reset the data source subset string

elpaso · github-actions[bot] · commit 6acb4149e430 · 2023-03-10T10:43:52.000Z
Fix #51934 by resetting the data source subset string filter, in case it was changed by a previous request.
diff --git a/src/core/providers/ogr/qgsogrfeatureiterator.cpp b/src/core/providers/ogr/qgsogrfeatureiterator.cpp
@@ -517,6 +517,13 @@ bool QgsOgrFeatureIterator::rewind()
 
 bool QgsOgrFeatureIterator::close()
 {
+  // Finally reset the data source filter, in case it was changed by a previous request
+  // this fixes https://github.com/qgis/QGIS/issues/51934
+  if ( mOgrLayer && ! mSource->mSubsetString.isEmpty() )
+  {
+    OGR_L_SetAttributeFilter( mOgrLayer,  mSource->mEncoding->fromUnicode( mSource->mSubsetString ).constData() );
+  }
+
   if ( mSharedDS )
   {
     iteratorClosed();
diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py
@@ -2638,6 +2638,59 @@ def testDateTimeTimeZoneMilliseconds(self):
         got = [feat for feat in vl.getFeatures()]
         self.assertEqual(got[0]["dt"], new_dt)
 
+    def testTransactionModeAutoWithFilter(self):
+
+        temp_dir = QTemporaryDir()
+        temp_path = temp_dir.path()
+        filename = os.path.join(temp_path, "test.gpkg")
+        ds = ogr.GetDriverByName("GPKG").CreateDataSource(filename)
+        lyr = ds.CreateLayer("points", geom_type=ogr.wkbPoint)
+        lyr.CreateField(ogr.FieldDefn('name', ogr.OFTString))
+        f = ogr.Feature(lyr.GetLayerDefn())
+        f['name'] = 'a'
+        f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(1 2)'))
+        lyr.CreateFeature(f)
+        f = None
+        ds = None
+        ds = None
+
+        vl = QgsVectorLayer(filename)
+        self.assertTrue(vl.isValid())
+        self.assertEqual(vl.featureCount(), 1)
+        self.assertEqual(next(vl.getFeatures())['name'], 'a')
+
+        p = QgsProject()
+        p.setAutoTransaction(True)
+        self.assertTrue(p.addMapLayers([vl]))
+        self.assertTrue(vl.setSubsetString('"name" IN (\'a\', \'b\')'))
+        self.assertEqual(vl.featureCount(), 1)
+        self.assertTrue(vl.startEditing())
+
+        # Add feature
+        f = QgsFeature(vl.fields())
+        f.setAttribute('name', 'b')
+        g = QgsGeometry.fromWkt('POINT(3 4)')
+        f.setGeometry(g)
+
+        # This triggers the issue because it sets the subset filter
+        req = QgsFeatureRequest()
+        req.setFilterExpression('"name" IS NULL')
+        self.assertEqual([f for f in vl.getFeatures(req)], [])
+
+        self.assertTrue(vl.addFeatures([f]))
+        self.assertTrue(vl.commitChanges())
+        self.assertEqual(vl.featureCount(), 2)
+        attrs = [f['name'] for f in vl.getFeatures()]
+        self.assertEqual(attrs, ['a', 'b'])
+
+        # verify
+        del p
+        vl2 = QgsVectorLayer(filename)
+        self.assertTrue(vl2.isValid())
+        self.assertEqual(vl2.featureCount(), 2)
+        attrs = [f['name'] for f in vl2.getFeatures()]
+        self.assertEqual(attrs, ['a', 'b'])
+
 
 if __name__ == '__main__':
     unittest.main()

Original file line number	Diff line number	Diff line change
`@@ -517,6 +517,13 @@ bool QgsOgrFeatureIterator::rewind()`
`517`	`517`
`518`	`518`	`bool QgsOgrFeatureIterator::close()`
`519`	`519`	`{`
	`520`	`+ // Finally reset the data source filter, in case it was changed by a previous request`
	`521`	`+ // this fixes https://github.com/qgis/QGIS/issues/51934`
	`522`	`+ if ( mOgrLayer && ! mSource->mSubsetString.isEmpty() )`
	`523`	`+ {`
	`524`	`+ OGR_L_SetAttributeFilter( mOgrLayer, mSource->mEncoding->fromUnicode( mSource->mSubsetString ).constData() );`
	`525`	`+ }`
	`526`	`+`
`520`	`527`	`if ( mSharedDS )`
`521`	`528`	`{`
`522`	`529`	`iteratorClosed();`