Skip to content

Commit fb49f70

Browse files
rouaultrouault
committedJun 1, 2016
[OGR provider] Fix deleteAttributes() when mFirstFieldIsFid
·
ltr-3_40archive/master_2
1 parent 65c3b96 commit fb49f70

File tree

3 files changed

+145
-81
lines changed

3 files changed

+145
-81
lines changed
 

‎src/providers/ogr/qgsogrprovider.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,11 +1287,18 @@ bool QgsOgrProvider::deleteAttributes( const QgsAttributeIds &attributes )
12871287
qSort( attrsLst.begin(), attrsLst.end(), qGreater<int>() );
12881288
Q_FOREACH ( int attr, attrsLst )
12891289
{
1290-
if ( attr == 0 && mFirstFieldIsFid )
1290+
if ( mFirstFieldIsFid )
12911291
{
1292-
pushError( tr( "Cannot delete feature id column" ) );
1293-
res = false;
1294-
break;
1292+
if ( attr == 0 )
1293+
{
1294+
pushError( tr( "Cannot delete feature id column" ) );
1295+
res = false;
1296+
break;
1297+
}
1298+
else
1299+
{
1300+
--attr;
1301+
}
12951302
}
12961303
if ( OGR_L_DeleteField( ogrLayer, attr ) != OGRERR_NONE )
12971304
{

‎tests/src/python/test_provider_ogr_gpkg.py

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -80,82 +80,5 @@ def testCurveGeometryType(self):
8080
# The geometries must be binarily identical
8181
self.assertEqual(got_geom.asWkb(), reference.asWkb(), 'Expected {}, got {}'.format(reference.exportToWkt(), got_geom.exportToWkt()))
8282

83-
def testFidSupport(self):
84-
85-
# We do not use @unittest.expectedFailure since the test might actually succeed
86-
# on Linux 64bit with GDAL 1.11, where "long" is 64 bit...
87-
# GDAL 2.0 is guaranteed to properly support it on all platforms
88-
version_num = int(gdal.VersionInfo('VERSION_NUM'))
89-
if version_num < GDAL_COMPUTE_VERSION(2, 0, 0):
90-
return
91-
92-
tmpfile = os.path.join(self.basetestpath, 'testFidSupport.gpkg')
93-
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
94-
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint)
95-
lyr.CreateField(ogr.FieldDefn('strfield', ogr.OFTString))
96-
f = ogr.Feature(lyr.GetLayerDefn())
97-
f.SetFID(12)
98-
f.SetField(0, 'foo')
99-
lyr.CreateFeature(f)
100-
f = None
101-
ds = None
102-
103-
vl = QgsVectorLayer(u'{}'.format(tmpfile), u'test', u'ogr')
104-
self.assertEqual(len(vl.fields()), 2)
105-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures()]
106-
self.assertEqual(got, [(12, 'foo')])
107-
108-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("strfield = 'foo'"))]
109-
self.assertEqual(got, [(12, 'foo')])
110-
111-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 12"))]
112-
self.assertEqual(got, [(12, 'foo')])
113-
114-
result = [f['strfield'] for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['strfield'], vl.dataProvider().fields()))]
115-
self.assertEqual(result, ['foo'])
116-
117-
result = [f['fid'] for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['fid'], vl.dataProvider().fields()))]
118-
self.assertEqual(result, [12])
119-
120-
# Test that when the 'fid' field is not set, regular insertion is done
121-
f = QgsFeature()
122-
f.setFields(vl.fields())
123-
f.setAttributes([None, 'automatic_id'])
124-
(res, out_f) = vl.dataProvider().addFeatures([f])
125-
self.assertEqual(out_f[0].id(), 13)
126-
self.assertEqual(out_f[0].attribute('fid'), 13)
127-
self.assertEqual(out_f[0].attribute('strfield'), 'automatic_id')
128-
129-
# Test that when the 'fid' field is set, it is really used to set the id
130-
f = QgsFeature()
131-
f.setFields(vl.fields())
132-
f.setAttributes([9876543210, 'bar'])
133-
(res, out_f) = vl.dataProvider().addFeatures([f])
134-
self.assertEqual(out_f[0].id(), 9876543210)
135-
self.assertEqual(out_f[0].attribute('fid'), 9876543210)
136-
self.assertEqual(out_f[0].attribute('strfield'), 'bar')
137-
138-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
139-
self.assertEqual(got, [(9876543210, 'bar')])
140-
141-
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {1: 'baz'}}))
142-
143-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
144-
self.assertEqual(got, [(9876543210, 'baz')])
145-
146-
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {0: 9876543210, 1: 'baw'}}))
147-
148-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
149-
self.assertEqual(got, [(9876543210, 'baw')])
150-
151-
# Not allowed: changing the fid regular field
152-
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {0: 12, 1: 'baw'}}))
153-
154-
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
155-
self.assertEqual(got, [(9876543210, 'baw')])
156-
157-
# Cannot delete fid
158-
self.assertFalse(vl.dataProvider().deleteAttributes([0]))
159-
16083
if __name__ == '__main__':
16184
unittest.main()

‎tests/src/python/test_provider_ogr_sqlite.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# -*- coding: utf-8 -*-
2+
"""QGIS Unit tests for the OGR/GPKG provider.
3+
4+
.. note:: This program is free software; you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License as published by
6+
the Free Software Foundation; either version 2 of the License, or
7+
(at your option) any later version.
8+
"""
9+
__author__ = 'Even Rouault'
10+
__date__ = '2016-06-01'
11+
__copyright__ = 'Copyright 2016, Even Rouault'
12+
# This will get replaced with a git SHA1 when you do a git archive
13+
__revision__ = '$Format:%H$'
14+
15+
import qgis # NOQA
16+
17+
import os
18+
import tempfile
19+
import shutil
20+
import glob
21+
from osgeo import gdal, ogr
22+
23+
from qgis.core import QgsVectorLayer, QgsFeature, QgsGeometry, QgsFeatureRequest
24+
from qgis.testing import start_app, unittest
25+
from utilities import unitTestDataPath
26+
27+
start_app()
28+
29+
30+
def GDAL_COMPUTE_VERSION(maj, min, rev):
31+
return ((maj) * 1000000 + (min) * 10000 + (rev) * 100)
32+
33+
34+
class TestPyQgsOGRProviderGpkg(unittest.TestCase):
Code has comments. Press enter to view.
35+
36+
@classmethod
37+
def setUpClass(cls):
38+
"""Run before all tests"""
39+
# Create test layer
40+
cls.basetestpath = tempfile.mkdtemp()
41+
42+
@classmethod
43+
def tearDownClass(cls):
44+
"""Run after all tests"""
45+
shutil.rmtree(cls.basetestpath, True)
46+
47+
def testFidSupport(self):
48+
49+
# We do not use @unittest.expectedFailure since the test might actually succeed
50+
# on Linux 64bit with GDAL 1.11, where "long" is 64 bit...
51+
# GDAL 2.0 is guaranteed to properly support it on all platforms
52+
version_num = int(gdal.VersionInfo('VERSION_NUM'))
53+
if version_num < GDAL_COMPUTE_VERSION(2, 0, 0):
54+
return
55+
56+
tmpfile = os.path.join(self.basetestpath, 'testFidSupport.sqlite')
57+
ds = ogr.GetDriverByName('SQLite').CreateDataSource(tmpfile)
58+
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint, options=['FID=fid'])
59+
lyr.CreateField(ogr.FieldDefn('strfield', ogr.OFTString))
60+
lyr.CreateField(ogr.FieldDefn('intfield', ogr.OFTInteger))
61+
f = ogr.Feature(lyr.GetLayerDefn())
62+
f.SetFID(12)
63+
f.SetField(0, 'foo')
64+
f.SetField(1, 123)
65+
lyr.CreateFeature(f)
66+
f = None
67+
ds = None
68+
69+
vl = QgsVectorLayer(u'{}'.format(tmpfile), u'test', u'ogr')
70+
self.assertEqual(len(vl.fields()), 3)
71+
got = [(f.attribute('fid'), f.attribute('strfield'), f.attribute('intfield')) for f in vl.getFeatures()]
72+
self.assertEqual(got, [(12, 'foo', 123)])
73+
74+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("strfield = 'foo'"))]
75+
self.assertEqual(got, [(12, 'foo')])
76+
77+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 12"))]
78+
self.assertEqual(got, [(12, 'foo')])
79+
80+
result = [f['strfield'] for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['strfield'], vl.dataProvider().fields()))]
81+
self.assertEqual(result, ['foo'])
82+
83+
result = [f['fid'] for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['fid'], vl.dataProvider().fields()))]
84+
self.assertEqual(result, [12])
85+
86+
# Test that when the 'fid' field is not set, regular insertion is done
87+
f = QgsFeature()
88+
f.setFields(vl.fields())
89+
f.setAttributes([None, 'automatic_id'])
90+
(res, out_f) = vl.dataProvider().addFeatures([f])
91+
self.assertEqual(out_f[0].id(), 13)
92+
self.assertEqual(out_f[0].attribute('fid'), 13)
93+
self.assertEqual(out_f[0].attribute('strfield'), 'automatic_id')
94+
95+
# Test that when the 'fid' field is set, it is really used to set the id
96+
f = QgsFeature()
97+
f.setFields(vl.fields())
98+
f.setAttributes([9876543210, 'bar'])
99+
(res, out_f) = vl.dataProvider().addFeatures([f])
100+
self.assertEqual(out_f[0].id(), 9876543210)
101+
self.assertEqual(out_f[0].attribute('fid'), 9876543210)
102+
self.assertEqual(out_f[0].attribute('strfield'), 'bar')
103+
104+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
105+
self.assertEqual(got, [(9876543210, 'bar')])
106+
107+
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {1: 'baz'}}))
108+
109+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
110+
self.assertEqual(got, [(9876543210, 'baz')])
111+
112+
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {0: 9876543210, 1: 'baw'}}))
113+
114+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
115+
self.assertEqual(got, [(9876543210, 'baw')])
116+
117+
# Not allowed: changing the fid regular field
118+
self.assertTrue(vl.dataProvider().changeAttributeValues({9876543210: {0: 12, 1: 'baw'}}))
119+
120+
got = [(f.attribute('fid'), f.attribute('strfield')) for f in vl.getFeatures(QgsFeatureRequest().setFilterExpression("fid = 9876543210"))]
121+
self.assertEqual(got, [(9876543210, 'baw')])
122+
123+
# Cannot delete fid
124+
self.assertFalse(vl.dataProvider().deleteAttributes([0]))
125+
126+
# Delete first "genuine" attribute
127+
self.assertTrue(vl.dataProvider().deleteAttributes([1]))
128+
129+
got = [(f.attribute('fid'), f.attribute('intfield')) for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setFilterExpression("fid = 12"))]
130+
self.assertEqual(got, [(12, 123)])
131+
132+
133+
if __name__ == '__main__':
134+
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.