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 afd9120 commit f16c28b0e38bbd613f35df2b826f6a2defdb5583
@@ -2763,8 +2763,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
{
@@ -1062,23 +1062,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())
@@ -111,7 +111,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 f16c28b

Please sign in to comment.