Skip to content
Permalink
Browse files

Fix missing features when combining FilterExpression requests with

subsets of attributes and the filter expression requires fields
which are not included in the attribute subset

Note: I've only fixed the providers which implement the vector
provider conformance unit tests. Other providers (eg GRASS) should
also implement a similar fix.
  • Loading branch information
nyalldawson committed Mar 30, 2016
1 parent 62c86f0 commit 402ee9d4cf6d026574a77cc503d8e26918daa78b
@@ -138,6 +138,19 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
mLoadGeometry = false;
}

// ensure that all attributes required for expression filter are being fetched
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes && request.filterType() == QgsFeatureRequest::FilterExpression )
{
QgsAttributeList attrs = request.subsetOfAttributes();
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !attrs.contains( attrIdx ) )
attrs << attrIdx;
}
mRequest.setSubsetOfAttributes( attrs );
}

QgsDebugMsg( QString( "Iterator is scanning file: " ) + ( mMode == FileScan ? "Yes" : "No" ) );
QgsDebugMsg( QString( "Iterator is loading geometries: " ) + ( mLoadGeometry ? "Yes" : "No" ) );
QgsDebugMsg( QString( "Iterator is testing geometries: " ) + ( mTestGeometry ? "Yes" : "No" ) );
@@ -77,7 +77,20 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
mAttributesToFetch.append( mFidCol );

bool subsetOfAttributes = mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes;
Q_FOREACH ( int i, subsetOfAttributes ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList() )
QgsAttributeList attrs = subsetOfAttributes ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList();

// ensure that all attributes required for expression filter are being fetched
if ( subsetOfAttributes && request.filterType() == QgsFeatureRequest::FilterExpression )
{
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !attrs.contains( attrIdx ) )
attrs << attrIdx;
}
}

Q_FOREACH ( int i, attrs )
{
QString fieldname = mSource->mFields.at( i ).name();
if ( mSource->mFidColName == fieldname )
@@ -64,6 +64,18 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
mFetchGeometry = ( !mRequest.filterRect().isNull() ) || !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
QgsAttributeList attrs = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList();

// ensure that all attributes required for expression filter are being fetched
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes && request.filterType() == QgsFeatureRequest::FilterExpression )
{
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !attrs.contains( attrIdx ) )
attrs << attrIdx;
}
mRequest.setSubsetOfAttributes( attrs );
}

// make sure we fetch just relevant fields
// unless it's a VRT data source filtered by geometry as we don't know which
// attributes make up the geometry and OGR won't fetch them to evaluate the
@@ -44,10 +44,23 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
mAttributeList = mRequest.subsetOfAttributes();
if ( mAttributeList.isEmpty() )
mAttributeList = mSource->mFields.allAttributesList();

// ensure that all attributes required for expression filter are being fetched
if ( mRequest.filterType() == QgsFeatureRequest::FilterExpression )
{
Q_FOREACH ( const QString& field, mRequest.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !mAttributeList.contains( attrIdx ) )
mAttributeList << attrIdx;
}
}

}
else
mAttributeList = mSource->mFields.allAttributesList();


QString whereClause;

if ( !mRequest.filterRect().isNull() && !mSource->mGeometryColumn.isNull() && mSource->mHasSpatialIndex )
@@ -89,6 +89,19 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
}
else if ( request.filterType() == QgsFeatureRequest::FilterExpression )
{
// ensure that all attributes required for expression filter are being fetched
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
{
QgsAttributeList attrs = mRequest.subsetOfAttributes();
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !attrs.contains( attrIdx ) )
attrs << attrIdx;
}
mRequest.setSubsetOfAttributes( attrs );
}

if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
//IMPORTANT - this MUST be the last clause added!
@@ -83,6 +83,19 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature
//IMPORTANT - this MUST be the last clause added!
else if ( request.filterType() == QgsFeatureRequest::FilterExpression )
{
// ensure that all attributes required for expression filter are being fetched
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes && request.filterType() == QgsFeatureRequest::FilterExpression )
{
QgsAttributeList attrs = request.subsetOfAttributes();
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mSource->mFields.fieldNameIndex( field );
if ( !attrs.contains( attrIdx ) )
attrs << attrIdx;
}
mRequest.setSubsetOfAttributes( attrs );
}

if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
QgsSpatiaLiteExpressionCompiler compiler = QgsSpatiaLiteExpressionCompiler( source );
@@ -80,12 +80,22 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
mFields = mSource->provider()->fields();
if ( request.flags() & QgsFeatureRequest::SubsetOfAttributes )
{

// copy only selected fields
Q_FOREACH ( int idx, request.subsetOfAttributes() )
{
mAttributes << idx;
}

// ensure that all attributes required for expression filter are being fetched
if ( request.filterType() == QgsFeatureRequest::FilterExpression )
{
Q_FOREACH ( const QString& field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mFields.fieldNameIndex( field );
if ( !mAttributes.contains( attrIdx ) )
mAttributes << attrIdx;
}
}
}
else
{
@@ -21,6 +21,10 @@ 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)

# Also check that filter works when referenced fields are not being retrieved by request
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setSubsetOfAttributes([0]))])
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}" using empty attribute subset'.format(set(expected), result, expression)

'''
This is a collection of tests for vector data providers and kept generic.
To make use of it, subclass it and set self.provider to a provider you want to test.

0 comments on commit 402ee9d

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