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
  • Loading branch information
nyalldawson committed May 16, 2016
1 parent c0214bc commit 858914e
Show file tree
Hide file tree
Showing 15 changed files with 48 additions and 10 deletions.
6 changes: 5 additions & 1 deletion src/providers/db2/qgsdb2featureiterator.cpp
Expand Up @@ -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
Expand Down
Expand Up @@ -129,6 +129,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 @@ -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 );
}
Expand Down
4 changes: 4 additions & 0 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -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
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 @@ -100,6 +101,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
}
mRequest.setSubsetOfAttributes( attrs );
}
mFilterRequiresGeometry = request.filterExpression()->needsGeometry();

if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
Expand Down Expand Up @@ -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 )
Expand Down
1 change: 1 addition & 0 deletions src/providers/postgres/qgspostgresfeatureiterator.h
Expand Up @@ -132,6 +132,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
11 changes: 9 additions & 2 deletions src/providers/wfs/qgswfsfeatureiterator.cpp
Expand Up @@ -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" ) );
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down
1 change: 1 addition & 0 deletions src/providers/wfs/qgswfsfeatureiterator.h
Expand Up @@ -241,6 +241,7 @@ class QgsWFSFeatureIterator : public QObject,
QString mReaderFilename;
QFile* mReaderFile;
QDataStream* mReaderStream;
bool mFetchGeometry;
};

/** Feature source */
Expand Down
5 changes: 4 additions & 1 deletion tests/src/python/providertestbase.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/src/python/test_provider_postgres.py
Expand Up @@ -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([])
Expand Down
3 changes: 2 additions & 1 deletion tests/src/python/test_provider_shapefile.py
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tests/src/python/test_provider_spatialite.py
Expand Up @@ -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%\'',
Expand Down

0 comments on commit 858914e

Please sign in to comment.