Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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 f16c28b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
3 changes: 2 additions & 1 deletion src/core/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -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
{
Expand Down
33 changes: 30 additions & 3 deletions tests/src/python/test_provider_ogr_gpkg.py
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion tests/src/python/test_provider_ogr_sqlite.py
Expand Up @@ -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')])
Expand Down

0 comments on commit f16c28b

Please sign in to comment.