Skip to content
Permalink
Browse files
[OGR provider] Do not corrupt values when updating a GPKG feature
and that the fid value is included in the fields to update, and that it
doesn't change.

Also rollback all changes if an update of FID is attempted.

Fixes #42274
  • Loading branch information
rouault authored and nyalldawson committed May 24, 2021
1 parent 75bb5d6 commit 852fcc4c1c6c0c00e1a018e62b7ec78b6cbab8cc
@@ -2385,8 +2385,9 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
if ( it2->toLongLong() != fid )
{
pushError( tr( "Changing feature id of feature %1 is not allowed." ).arg( fid ) );
continue;
returnValue = false;
}
continue;
}
else
{
@@ -1059,23 +1059,50 @@ def testGeopackageLargeFID(self):
vl = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
f = QgsFeature()
f.setAttributes([1234567890123, None])
f2 = QgsFeature()
f2.setAttributes([1234567890124, None])
self.assertTrue(vl.startEditing())
self.assertTrue(vl.dataProvider().addFeatures([f]))
self.assertTrue(vl.dataProvider().addFeatures([f, f2]))
self.assertTrue(vl.commitChanges())

got = [feat for feat in vl.getFeatures()][0]
got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890123))][0]
self.assertEqual(got['fid'], 1234567890123)

self.assertTrue(vl.startEditing())
self.assertTrue(vl.changeGeometry(1234567890123, QgsGeometry.fromWkt('Point (3 50)')))
self.assertTrue(vl.changeAttributeValue(1234567890123, 1, 'foo'))
self.assertTrue(vl.commitChanges())

got = [feat for feat in vl.getFeatures()][0]
got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890123))][0]
self.assertEqual(got['str_field'], 'foo')
got_geom = got.geometry()
self.assertIsNotNone(got_geom)

# We don't change the FID, so OK
self.assertTrue(vl.startEditing())
self.assertTrue(vl.dataProvider().changeAttributeValues({1234567890123: {0: 1234567890123, 1: 'bar'},
1234567890124: {0: 1234567890124, 1: 'bar2'}}))
self.assertTrue(vl.commitChanges())

got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890123))][0]
self.assertEqual(got['str_field'], 'bar')

got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890124))][0]
self.assertEqual(got['str_field'], 'bar2')

# We try to change the FID, not allowed
# also check that all changes where reverted
self.assertTrue(vl.startEditing())
self.assertFalse(vl.dataProvider().changeAttributeValues({1234567890123: {0: 1, 1: 'baz'},
1234567890124: {1: 'baz2'}}))
self.assertTrue(vl.commitChanges())

got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890123))][0]
self.assertEqual(got['str_field'], 'bar')

got = [feat for feat in vl.getFeatures(QgsFeatureRequest(1234567890124))][0]
self.assertEqual(got['str_field'], 'bar2')

self.assertTrue(vl.startEditing())
self.assertTrue(vl.deleteFeature(1234567890123))
self.assertTrue(vl.commitChanges())
@@ -119,7 +119,7 @@ def testFidSupport(self):
self.assertEqual(got, [(9876543210, 'baw')])

# Not allowed: changing the fid regular field
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {0: 12, 1: 'baw'}}))
self.assertFalse(vl.dataProvider().changeAttributeValues({9876543210: {0: 12, 1: 'baw'}}))

got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
self.assertEqual(got, [(9876543210, 'baw')])

0 comments on commit 852fcc4

Please sign in to comment.