Ensure that providers fetch geometry for a QgsFeatureRequest

with an expression filter which requires geometry
qgis · May 16, 2016 · 858914e · 858914e
1 parent c0214bc
commit 858914e
Show file tree

Hide file tree

Showing 15 changed files with 48 additions and 10 deletions.
diff --git a/src/providers/db2/qgsdb2featureiterator.cpp b/src/providers/db2/qgsdb2featureiterator.cpp
@@ -102,7 +102,11 @@ void QgsDb2FeatureIterator::BuildStatement( const QgsFeatureRequest& request )
   }
 
   // get geometry col if requested and table has spatial column
-  if ( !( request.flags() & QgsFeatureRequest::NoGeometry ) && mSource->isSpatial() )
+  if ((
+        !( request.flags() & QgsFeatureRequest::NoGeometry )
+        || ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
+      )
+      && mSource->isSpatial() )
   {
     mStatement += QString( delim + "DB2GSE.ST_ASBINARY(%1) AS %1 " ).arg( mSource->mGeometryColName );
     mAttributesToFetch.append( 2 );  // dummy - won't store geometry as an attribute

diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp
@@ -129,6 +129,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
          !( mRequest.flags() & QgsFeatureRequest::NoGeometry )
          || mTestGeometry
          || ( mTestSubset && mSource->mSubsetExpression->needsGeometry() )
+         || ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() )
        )
      )
   {

diff --git a/src/providers/mssql/qgsmssqlfeatureiterator.cpp b/src/providers/mssql/qgsmssqlfeatureiterator.cpp
@@ -101,7 +101,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 );
   }

diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp
@@ -89,6 +89,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

diff --git a/src/providers/oracle/qgsoraclefeatureiterator.cpp b/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() )
     {

diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -37,6 +37,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
     , mExpressionCompiled( false )
     , mOrderByCompiled( false )
     , mLastFetch( false )
+    , mFilterRequiresGeometry( false )
 {
   if ( !source->mTransactionConnection )
   {
@@ -100,6 +101,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
       }
       mRequest.setSubsetOfAttributes( attrs );
     }
+    mFilterRequiresGeometry = request.filterExpression()->needsGeometry();
 
     if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
     {
@@ -452,7 +454,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 )

diff --git a/src/providers/postgres/qgspostgresfeatureiterator.h b/src/providers/postgres/qgspostgresfeatureiterator.h
@@ -132,6 +132,7 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Q
     bool mExpressionCompiled;
     bool mOrderByCompiled;
     bool mLastFetch;
+    bool mFilterRequiresGeometry;
 };
 
 #endif // QGSPOSTGRESFEATUREITERATOR_H
diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/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() )
     {

diff --git a/src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp b/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() );
     }

diff --git a/src/providers/wfs/qgswfsfeatureiterator.cpp b/src/providers/wfs/qgswfsfeatureiterator.cpp
@@ -712,6 +712,7 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource* source,
     , mWriterStream( nullptr )
     , mReaderFile( nullptr )
     , mReaderStream( nullptr )
+    , mFetchGeometry( false )
 {
   // Configurable for the purpose of unit tests
   QString threshold( getenv( "QGIS_WFS_ITERATOR_TRANSFER_THRESHOLD" ) );
@@ -744,6 +745,12 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource* source,
 
   requestCache.setFilterRect( mRequest.filterRect() );
 
+  if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) ||
+       ( mRequest.filterType() == QgsFeatureRequest::FilterExpression && mRequest.filterExpression()->needsGeometry() ) )
+  {
+    mFetchGeometry = true;
+  }
+
   if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
   {
     const QgsFields & dataProviderFields = mShared->mCacheDataProvider->fields();
@@ -772,7 +779,7 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource* source,
       }
     }
 
-    if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+    if ( mFetchGeometry )
     {
       int hexwkbGeomIdx = dataProviderFields.indexFromName( QgsWFSConstants::FIELD_HEXWKB_GEOM );
       Q_ASSERT( hexwkbGeomIdx >= 0 );
@@ -906,7 +913,7 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature& f )
 
     //QgsDebugMsg(QString("QgsWFSSharedData::fetchFeature() : mCacheIterator.nextFeature(cachedFeature)") );
 
-    if ( !mShared->mGeometryAttribute.isEmpty() && !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+    if ( !mShared->mGeometryAttribute.isEmpty() && mFetchGeometry )
     {
       int idx = cachedFeature.fields()->indexFromName( QgsWFSConstants::FIELD_HEXWKB_GEOM );
       Q_ASSERT( idx >= 0 );

diff --git a/src/providers/wfs/qgswfsfeatureiterator.h b/src/providers/wfs/qgswfsfeatureiterator.h
@@ -241,6 +241,7 @@ class QgsWFSFeatureIterator : public QObject,
     QString mReaderFilename;
     QFile* mReaderFile;
     QDataStream* mReaderStream;
+    bool mFetchGeometry;
 };
 
 /** Feature source */

diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -75,7 +75,7 @@ def partiallyCompiledFilters(self):
         return set()
 
     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)
 
         if self.compiled:
@@ -183,6 +183,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):
         self.compiled = False
         try:

diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -60,7 +60,7 @@ def disableCompiler(self):
         QSettings().setValue(u'/qgis/compileExpressions', False)
 
     def uncompiledFilters(self):
-        return set([])
+        return set(['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))\'))'])
 
     def partiallyCompiledFilters(self):
         return set([])

diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -111,7 +111,8 @@ def uncompiledFilters(self):
                        '-cnt < 0',
                        '-cnt - 1 = -101',
                        '-(-cnt) = 100',
-                       '-(cnt) = -(100)'])
+                       '-(cnt) = -(100)',
+                       '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))\'))'])
         if int(osgeo.gdal.VersionInfo()[:1]) < 2:
             filters.insert('not null')
         return filters

diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py
@@ -130,7 +130,8 @@ def disableCompiler(self):
 
     def uncompiledFilters(self):
         return set(['cnt = 10 ^ 2',
-                    '"name" ~ \'[OP]ra[gne]+\''])
+                    '"name" ~ \'[OP]ra[gne]+\'',
+                    '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))\'))'])
 
     def partiallyCompiledFilters(self):
         return set(['"name" NOT LIKE \'Ap%\'',