Ensure that providers fetch geometry for a QgsFeatureRequest

with an expression filter which requires geometry

(cherry-picked from 858914e)

Author: nyalldawson

src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp

@@ -128,6 +128,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
          !( mRequest.flags() & QgsFeatureRequest::NoGeometry )
          || mTestGeometry
          || ( mTestSubset && mSource->mSubsetExpression->needsGeometry() )
+         || ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
        )
      )
   {

src/providers/mssql/qgsmssqlfeatureiterator.cpp

@@ -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 );
   }

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -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

src/providers/oracle/qgsoraclefeatureiterator.cpp

@@ -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() )
     {

src/providers/postgres/qgspostgresfeatureiterator.cpp

@@ -37,6 +37,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
     , mExpressionCompiled( false )
     , mOrderByCompiled( false )
     , mLastFetch( false )
+    , mFilterRequiresGeometry( false )
 {
   if ( !source->mTransactionConnection )
   {
@@ -101,6 +102,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
       }
       mRequest.setSubsetOfAttributes( attrs );
     }
+    mFilterRequiresGeometry = request.filterExpression()->needsGeometry();
 
     if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
     {
@@ -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 )

src/providers/postgres/qgspostgresfeatureiterator.h

@@ -129,6 +129,7 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Q
     bool mExpressionCompiled;
     bool mOrderByCompiled;
     bool mLastFetch;
+    bool mFilterRequiresGeometry;
 };
 
 #endif // QGSPOSTGRESFEATUREITERATOR_H

src/providers/spatialite/qgsspatialitefeatureiterator.cpp

@@ -95,6 +95,10 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature
       }
       mRequest.setSubsetOfAttributes( attrs );
     }
+    if ( request.filterExpression()->needsGeometry() )
+    {
+      mFetchGeometry = true;
+    }
 
     if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
     {

src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp

@@ -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() );
     }

tests/src/python/providertestbase.py

@@ -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
@@ -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()