Skip to content
Permalink
Browse files

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
  • Loading branch information
nyalldawson committed Apr 13, 2016
1 parent ecc85a9 commit 02e0e3f95925356c4da8ed5effa4634e92fd44f8
@@ -360,8 +360,13 @@ bool QgsDb2FeatureIterator::fetchFeature( QgsFeature& feature )
else
{
QgsDebugMsg( "Geometry is empty" );
feature.setGeometry( nullptr );
}
}
else
{
feature.setGeometry( nullptr );
}
feature.setValid( true );
mFetchCount++;
if ( mFetchCount % 100 == 0 )
@@ -56,19 +56,20 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
QgsRectangle rect = request.filterRect();

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

else if ( rect.contains( mSource->mExtent ) )
else if ( rect.contains( mSource->mExtent ) && !mTestSubset )
{
QgsDebugMsg( "Rectangle contains layer extents - bypass spatial filter" );
mTestGeometry = false;
}

// If we have a spatial index then use it. The spatial index already accounts
// for the subset. Also means we don't have to test geometries unless doing exact
// intersection
@@ -332,9 +333,7 @@ bool QgsDelimitedTextFeatureIterator::nextFeatureInternal( QgsFeature& feature )
feature.setFields( mSource->mFields ); // allow name-based attribute lookups
feature.setFeatureId( fid );
feature.initAttributes( mSource->mFields.count() );

if ( geom )
feature.setGeometry( geom );
feature.setGeometry( geom );

// If we are testing subset expression, then need all attributes just in case.
// Could be more sophisticated, but probably not worth it!
@@ -779,7 +779,7 @@ void QgsDelimitedTextProvider::rescanFile()
bool foundFirstGeometry = false;
while ( fi.nextFeature( f ) )
{
if ( mGeometryType != QGis::NoGeometry )
if ( mGeometryType != QGis::NoGeometry && f.constGeometry() )
{
if ( !foundFirstGeometry )
{
@@ -315,6 +315,14 @@ bool QgsMssqlFeatureIterator::fetchFeature( QgsFeature& feature )
g->fromWkb( wkb, mParser.GetWkbLen() );
feature.setGeometry( g );
}
else
{
feature.setGeometry( nullptr );
}
}
else
{
feature.setGeometry( nullptr );
}

feature.setValid( true );
@@ -228,6 +228,10 @@ bool QgsVirtualLayerFeatureIterator::fetchFeature( QgsFeature& feature )
{
feature.setGeometry( spatialiteBlobToQgsGeometry( blob.constData(), blob.size() ) );
}
else
{
feature.setGeometry( nullptr );
}
}

feature.setValid( true );
@@ -14,9 +14,53 @@

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

from utilities import(
compareWkt
)


class ProviderTestCase(object):

def testGetFeatures(self):
""" Test that expected results are returned when fetching all features """

# IMPORTANT - we do not use `for f in provider.getFeatures()` as we are also
# testing that existing attributes & geometry in f are overwritten correctly
# (for f in ... uses a new QgsFeature for every iteration)

it = self.provider.getFeatures()
f = QgsFeature()
attributes = {}
geometries = {}
while it.nextFeature(f):
# split off the first 5 attributes only - some provider test datasets will include
# additional attributes which we ignore
attrs = f.attributes()[0:5]
# force the num_char attribute to be text - some providers (eg delimited text) will
# automatically detect that this attribute contains numbers and set it as a numeric
# field
attrs[4] = str(attrs[4])
attributes[f['pk']] = attrs
geometries[f['pk']] = f.constGeometry() and f.constGeometry().exportToWkt()

expected_attributes = {5: [5, -200, NULL, 'NuLl', '5'],
3: [3, 300, 'Pear', 'PEaR', '3'],
1: [1, 100, 'Orange', 'oranGe', '1'],
2: [2, 200, 'Apple', 'Apple', '2'],
4: [4, 400, 'Honey', 'Honey', '4']}
self.assertEqual(attributes, expected_attributes, 'Expected {}, got {}'.format(expected_attributes, attributes))

expected_geometries = {1: 'Point (-70.332 66.33)',
2: 'Point (-68.2 70.8)',
3: None,
4: 'Point(-65.32 78.3)',
5: 'Point(-71.123 78.23)'}
for pk, geom in expected_geometries.iteritems():
if geom:
assert compareWkt(geom, geometries[pk]), "Geometry {} mismatch Expected:\n{}\nGot:\n{}\n".format(pk, geom, geometries[pk].exportToWkt())
else:
self.assertFalse(geometries[pk], 'Expected null geometry for {}'.format(pk))

def assert_query(self, provider, expression, expected):
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression))])
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}"'.format(set(expected), result, expression)

0 comments on commit 02e0e3f

Please sign in to comment.
You can’t perform that action at this time.