Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Ensure that providers fetch geometry for a QgsFeatureRequest
with an expression filter which requires geometry

(cherry-picked from 858914e)
  • Loading branch information
nyalldawson committed May 19, 2016
1 parent e3556f7 commit a2756b9
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 4 deletions.
Expand Up @@ -128,6 +128,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
!( mRequest.flags() & QgsFeatureRequest::NoGeometry )
|| mTestGeometry
|| ( mTestSubset && mSource->mSubsetExpression->needsGeometry() )
|| ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
)
)
{
Expand Down
5 changes: 4 additions & 1 deletion src/providers/mssql/qgsmssqlfeatureiterator.cpp
Expand Up @@ -102,7 +102,10 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
}

// get geometry col
if ( !( request.flags() & QgsFeatureRequest::NoGeometry ) && mSource->isSpatial() )
if (( !( request.flags() & QgsFeatureRequest::NoGeometry )
|| ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
)
&& mSource->isSpatial() )
{
mStatement += QString( ",[%1]" ).arg( mSource->mGeometryColName );
}
Expand Down
4 changes: 4 additions & 0 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -74,6 +74,10 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
}
mRequest.setSubsetOfAttributes( attrs );
}
if ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
{
mFetchGeometry = true;
}

// make sure we fetch just relevant fields
// unless it's a VRT data source filtered by geometry as we don't know which
Expand Down
4 changes: 4 additions & 0 deletions src/providers/oracle/qgsoraclefeatureiterator.cpp
Expand Up @@ -68,6 +68,10 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
{
// fetch geometry if requested
mFetchGeometry = ( mRequest.flags() & QgsFeatureRequest::NoGeometry ) == 0;
if ( mRequest.filterType() == QgsFeatureRequest::FilterExpression && mRequest.filterExpression()->needsGeometry() )
{
mFetchGeometry = true;
}

if ( !mRequest.filterRect().isNull() )
{
Expand Down
4 changes: 3 additions & 1 deletion src/providers/postgres/qgspostgresfeatureiterator.cpp
Expand Up @@ -37,6 +37,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
, mExpressionCompiled( false )
, mOrderByCompiled( false )
, mLastFetch( false )
, mFilterRequiresGeometry( false )
{
if ( !source->mTransactionConnection )
{
Expand Down Expand Up @@ -101,6 +102,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
}
mRequest.setSubsetOfAttributes( attrs );
}
mFilterRequiresGeometry = request.filterExpression()->needsGeometry();

if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
Expand Down Expand Up @@ -436,7 +438,7 @@ QString QgsPostgresFeatureIterator::whereClauseRect()

bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause, long limit, bool closeOnFail, const QString& orderBy )
{
mFetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && !mSource->mGeometryColumn.isNull();
mFetchGeometry = ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) || mFilterRequiresGeometry ) && !mSource->mGeometryColumn.isNull();
#if 0
// TODO: check that all field indexes exist
if ( !hasAllFields )
Expand Down
1 change: 1 addition & 0 deletions src/providers/postgres/qgspostgresfeatureiterator.h
Expand Up @@ -129,6 +129,7 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Q
bool mExpressionCompiled;
bool mOrderByCompiled;
bool mLastFetch;
bool mFilterRequiresGeometry;
};

#endif // QGSPOSTGRESFEATUREITERATOR_H
4 changes: 4 additions & 0 deletions src/providers/spatialite/qgsspatialitefeatureiterator.cpp
Expand Up @@ -95,6 +95,10 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature
}
mRequest.setSubsetOfAttributes( attrs );
}
if ( request.filterExpression()->needsGeometry() )
{
mFetchGeometry = true;
}

if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
Expand Down
4 changes: 3 additions & 1 deletion src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp
Expand Up @@ -121,7 +121,9 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
}
}
// the last column is the geometry, if any
if ( !( request.flags() & QgsFeatureRequest::NoGeometry ) && !mDefinition.geometryField().isNull() && mDefinition.geometryField() != "*no*" )
if (( !( request.flags() & QgsFeatureRequest::NoGeometry )
|| ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() ) )
&& !mDefinition.geometryField().isNull() && mDefinition.geometryField() != "*no*" )
{
columns += "," + quotedColumn( mDefinition.geometryField() );
}
Expand Down
5 changes: 4 additions & 1 deletion tests/src/python/providertestbase.py
Expand Up @@ -62,7 +62,7 @@ def testGetFeatures(self):
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))])
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setFlags(QgsFeatureRequest.NoGeometry))])
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
Expand Down Expand Up @@ -154,6 +154,9 @@ def runGetFeatureTests(self, provider):
# against numeric literals
self.assert_query(provider, 'num_char IN (2, 4, 5)', [2, 4, 5])

# geometry
self.assert_query(provider, 'intersects($geometry,geom_from_wkt( \'Polygon ((-72.2 66.1, -65.2 66.1, -65.2 72.0, -72.2 72.0, -72.2 66.1))\'))', [1, 2])

def testGetFeaturesUncompiled(self):
try:
self.disableCompiler()
Expand Down

0 comments on commit a2756b9

Please sign in to comment.