Skip to content

Commit 6880b53

Browse files
nyalldawsonnyalldawson
committedApr 15, 2016
Fix other feature's geometries are shown instead of null geometry
in delimited text provider (fix #14666) Add tests, also fix virtual layer, mssql and db2 providers which suffered the same bug (cherry-picked from 02e0e3f)
·
final-2_14_22final-2_14_2
1 parent 0161497 commit 6880b53

File tree

7 files changed

+122
-6
lines changed

7 files changed

+122
-6
lines changed
 

‎src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,20 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
5656
QgsRectangle rect = request.filterRect();
5757

5858
// If request doesn't overlap extents, then nothing to return
59-
if ( ! rect.intersects( mSource->mExtent ) )
59+
if ( ! rect.intersects( mSource->mExtent ) && !mTestSubset )
6060
{
6161
QgsDebugMsg( "Rectangle outside layer extents - no features to return" );
6262
mMode = FeatureIds;
6363
}
6464
// If the request extents include the entire layer, then revert to
6565
// a file scan
6666

67-
else if ( rect.contains( mSource->mExtent ) )
67+
else if ( rect.contains( mSource->mExtent ) && !mTestSubset )
6868
{
6969
QgsDebugMsg( "Rectangle contains layer extents - bypass spatial filter" );
7070
mTestGeometry = false;
7171
}
72+
7273
// If we have a spatial index then use it. The spatial index already accounts
7374
// for the subset. Also means we don't have to test geometries unless doing exact
7475
// intersection
@@ -332,9 +333,7 @@ bool QgsDelimitedTextFeatureIterator::nextFeatureInternal( QgsFeature& feature )
332333
feature.setFields( mSource->mFields ); // allow name-based attribute lookups
333334
feature.setFeatureId( fid );
334335
feature.initAttributes( mSource->mFields.count() );
335-
336-
if ( geom )
337-
feature.setGeometry( geom );
336+
feature.setGeometry( geom );
338337

339338
// If we are testing subset expression, then need all attributes just in case.
340339
// Could be more sophisticated, but probably not worth it!

‎src/providers/delimitedtext/qgsdelimitedtextprovider.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ void QgsDelimitedTextProvider::rescanFile()
779779
bool foundFirstGeometry = false;
780780
while ( fi.nextFeature( f ) )
781781
{
782-
if ( mGeometryType != QGis::NoGeometry )
782+
if ( mGeometryType != QGis::NoGeometry && f.constGeometry() )
783783
{
784784
if ( !foundFirstGeometry )
785785
{

‎src/providers/mssql/qgsmssqlfeatureiterator.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,14 @@ bool QgsMssqlFeatureIterator::fetchFeature( QgsFeature& feature )
315315
g->fromWkb( wkb, mParser.GetWkbLen() );
316316
feature.setGeometry( g );
317317
}
318+
else
319+
{
320+
feature.setGeometry( nullptr );
321+
}
322+
}
323+
else
324+
{
325+
feature.setGeometry( nullptr );
318326
}
319327

320328
feature.setValid( true );

‎src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,10 @@ bool QgsVirtualLayerFeatureIterator::fetchFeature( QgsFeature& feature )
228228
{
229229
feature.setGeometry( spatialiteBlobToQgsGeometry( blob.constData(), blob.size() ) );
230230
}
231+
else
232+
{
233+
feature.setGeometry( nullptr );
234+
}
231235
}
232236

233237
feature.setValid( true );

‎tests/src/python/providertestbase.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,53 @@
1414

1515
from qgis.core import QgsRectangle, QgsFeatureRequest, QgsFeature, QgsGeometry, NULL
1616

17+
from utilities import(
18+
compareWkt
19+
)
20+
1721

1822
class ProviderTestCase(object):
1923

24+
def testGetFeatures(self):
25+
""" Test that expected results are returned when fetching all features """
26+
27+
# IMPORTANT - we do not use `for f in provider.getFeatures()` as we are also
28+
# testing that existing attributes & geometry in f are overwritten correctly
29+
# (for f in ... uses a new QgsFeature for every iteration)
30+
31+
it = self.provider.getFeatures()
32+
f = QgsFeature()
33+
attributes = {}
34+
geometries = {}
35+
while it.nextFeature(f):
36+
# split off the first 5 attributes only - some provider test datasets will include
37+
# additional attributes which we ignore
38+
attrs = f.attributes()[0:5]
39+
# force the num_char attribute to be text - some providers (eg delimited text) will
40+
# automatically detect that this attribute contains numbers and set it as a numeric
41+
# field
42+
attrs[4] = str(attrs[4])
43+
attributes[f['pk']] = attrs
44+
geometries[f['pk']] = f.constGeometry() and f.constGeometry().exportToWkt()
45+
46+
expected_attributes = {5: [5, -200, NULL, 'NuLl', '5'],
47+
3: [3, 300, 'Pear', 'PEaR', '3'],
48+
1: [1, 100, 'Orange', 'oranGe', '1'],
49+
2: [2, 200, 'Apple', 'Apple', '2'],
50+
4: [4, 400, 'Honey', 'Honey', '4']}
51+
self.assertEqual(attributes, expected_attributes, 'Expected {}, got {}'.format(expected_attributes, attributes))
52+
53+
expected_geometries = {1: 'Point (-70.332 66.33)',
54+
2: 'Point (-68.2 70.8)',
55+
3: None,
56+
4: 'Point(-65.32 78.3)',
57+
5: 'Point(-71.123 78.23)'}
58+
for pk, geom in expected_geometries.iteritems():
59+
if geom:
60+
assert compareWkt(geom, geometries[pk]), "Geometry {} mismatch Expected:\n{}\nGot:\n{}\n".format(pk, geom, geometries[pk].exportToWkt())
61+
else:
62+
self.assertFalse(geometries[pk], 'Expected null geometry for {}'.format(pk))
63+
2064
def assert_query(self, provider, expression, expected):
2165
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression))])
2266
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}"'.format(set(expected), result, expression)

‎tests/src/python/test_qgsdelimitedtextprovider_wanted.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,3 +2395,57 @@ def test_039_issue_13749():
23952395
u'1 records have missing geometry definitions',
23962396
]
23972397
return wanted
2398+
2399+
2400+
def test_040_issue_14666():
2401+
wanted = {}
2402+
wanted['uri'] = u'file://test14666.csv?yField=y&xField=x&type=csv&delimiter=\\t'
2403+
wanted['fieldTypes'] = ['integer', 'double', 'double']
2404+
wanted['geometryType'] = 0
2405+
wanted['data'] = {
2406+
2: {
2407+
'id': u'1',
2408+
'description': u'7.15417',
2409+
'x': u'7.15417',
2410+
'y': u'50.680622',
2411+
'#fid': 2,
2412+
'#geometry': 'Point (7.1541699999999997 50.68062199999999962)',
2413+
},
2414+
3: {
2415+
'id': u'2',
2416+
'description': u'7.119219',
2417+
'x': u'7.119219',
2418+
'y': u'50.739814',
2419+
'#fid': 3,
2420+
'#geometry': 'Point (7.11921900000000019 50.73981400000000264)',
2421+
},
2422+
4: {
2423+
'id': u'3',
2424+
'description': u'NULL',
2425+
'x': u'NULL',
2426+
'y': u'NULL',
2427+
'#fid': 4,
2428+
'#geometry': 'None',
2429+
},
2430+
5: {
2431+
'id': u'4',
2432+
'description': u'NULL',
2433+
'x': u'NULL',
2434+
'y': u'NULL',
2435+
'#fid': 5,
2436+
'#geometry': 'None',
2437+
},
2438+
6: {
2439+
'id': u'5',
2440+
'description': u'7.129229',
2441+
'x': u'7.129229',
2442+
'y': u'50.703692',
2443+
'#fid': 6,
2444+
'#geometry': 'Point (7.12922899999999959 50.70369199999999665)',
2445+
},
2446+
}
2447+
wanted['log'] = [
2448+
u'Errors in file test14666.csv',
2449+
u'2 records have missing geometry definitions',
2450+
]
2451+
return wanted

‎tests/testdata/delimitedtext/test14666.csv

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
id x y
2+
1 7.15417 50.680622
3+
2 7.119219 50.739814
4+
3
5+
4
6+
5 7.129229 50.703692
7+

0 commit comments

Comments
 (0)
Please sign in to comment.