From 5be0ee734fa233f2a3f893ddc72e8264d029f2d0 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 14 Jul 2015 15:35:08 +0200 Subject: [PATCH] Remove QgsFeatureRequest::FilterRect from providers/iterators --- python/core/qgsvectorlayer.sip | 7 +- src/core/qgsvectorlayer.cpp | 5 -- src/core/qgsvectorlayer.h | 9 ++- src/core/qgsvectorlayerfeatureiterator.cpp | 15 ++-- src/core/qgsvectorlayerrenderer.cpp | 1 + .../qgsdelimitedtextfeatureiterator.cpp | 42 ++++++----- src/providers/gpx/qgsgpxfeatureiterator.cpp | 6 +- .../grass/qgsgrassfeatureiterator.cpp | 2 +- .../memory/qgsmemoryfeatureiterator.cpp | 8 +-- .../mssql/qgsmssqlfeatureiterator.cpp | 2 +- src/providers/ogr/qgsogrfeatureiterator.cpp | 8 +-- .../oracle/qgsoraclefeatureiterator.cpp | 70 ++++++++++--------- src/providers/oracle/qgsoracleprovider.cpp | 9 +++ src/providers/oracle/qgsoracleprovider.h | 2 + .../qgspostgresexpressioncompiler.cpp | 25 ++++++- .../postgres/qgspostgresfeatureiterator.cpp | 15 ++-- .../postgres/qgspostgresprovider.cpp | 10 +++ src/providers/postgres/qgspostgresprovider.h | 2 + .../qgsspatialitefeatureiterator.cpp | 5 +- src/providers/wfs/qgswfsfeatureiterator.cpp | 26 ++++--- tests/src/python/providertestbase.py | 9 +++ 21 files changed, 174 insertions(+), 104 deletions(-) diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip index 53adde3ed43..2dc54da1ca9 100644 --- a/python/core/qgsvectorlayer.sip +++ b/python/core/qgsvectorlayer.sip @@ -801,7 +801,12 @@ class QgsVectorLayer : QgsMapLayer /** Return the extent of the layer as a QRect */ QgsRectangle extent(); - /** Returns field list in the to-be-committed state */ + /** + * Returns the list of fields of this layer. + * This also includes fields which have not yet been saved to the provider. + * + * @return A list of fields + */ const QgsFields &pendingFields() const; /** Returns list of attributes */ diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index af4ada2d611..a2acd51943f 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/src/core/qgsvectorlayer.cpp @@ -2220,11 +2220,6 @@ bool QgsVectorLayer::deleteFeature( QgsFeatureId fid ) return res; } -const QgsFields &QgsVectorLayer::pendingFields() const -{ - return mUpdatedFields; -} - QgsAttributeList QgsVectorLayer::pendingAllAttributesList() { return mUpdatedFields.allAttributesList(); diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h index 997fd8f72db..2da408e5531 100644 --- a/src/core/qgsvectorlayer.h +++ b/src/core/qgsvectorlayer.h @@ -1300,8 +1300,13 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer /** Return the extent of the layer as a QRect */ QgsRectangle extent() override; - /** Returns field list in the to-be-committed state */ - const QgsFields &pendingFields() const; + /** + * Returns the list of fields of this layer. + * This also includes fields which have not yet been saved to the provider. + * + * @return A list of fields + */ + const QgsFields &pendingFields() const { return mUpdatedFields; } /** Returns list of attributes */ QgsAttributeList pendingAllAttributesList(); diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp index e5a767faf5e..71625daee81 100644 --- a/src/core/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/qgsvectorlayerfeatureiterator.cpp @@ -176,7 +176,7 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature& f ) return res; } - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) + if ( !mRequest.filterRect().isNull() ) { if ( fetchNextChangedGeomFeature( f ) ) return true; @@ -352,6 +352,10 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedAttributeFeature( QgsFeature { while ( mChangedFeaturesIterator.nextFeature( f ) ) { + if ( mFetchConsidered.contains( f.id() ) ) + // skip deleted features and those already handled by the geometry + continue; + mFetchConsidered << f.id(); updateChangedAttributes( f ); @@ -359,14 +363,7 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedAttributeFeature( QgsFeature if ( mHasVirtualAttributes ) addVirtualAttributes( f ); - if ( mRequest.filterType() == QgsFeatureRequest::FilterExpression ) - { - if ( mRequest.filterExpression()->evaluate( &f ).toBool() ) - { - return true; - } - } - else + if ( mRequest.filterExpression()->evaluate( &f ).toBool() ) { return true; } diff --git a/src/core/qgsvectorlayerrenderer.cpp b/src/core/qgsvectorlayerrenderer.cpp index 9c4d5b0dce7..db62be63959 100644 --- a/src/core/qgsvectorlayerrenderer.cpp +++ b/src/core/qgsvectorlayerrenderer.cpp @@ -136,6 +136,7 @@ bool QgsVectorLayerRenderer::render() QgsRenderOptions opts = mRendererV2->startRender( mContext, mFields ); + QgsDebugMsg( opts.whereClause() ); QgsRectangle requestExtent = mContext.extent(); mRendererV2->modifyRequestExtent( requestExtent, mContext ); diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp index 38a1a3179a7..5d76c0adeb1 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp +++ b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp @@ -44,21 +44,8 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe mTestGeometry = false; mMode = FileScan; - if ( request.filterType() == QgsFeatureRequest::FilterFid ) - { - QgsDebugMsg( "Configuring for returning single id" ); - mFeatureIds.append( request.filterFid() ); - mMode = FeatureIds; - mTestSubset = false; - } - // If have geometry and testing geometry then evaluate options... - // If we don't have geometry then all records pass geometry filter. - // CC: 2013-05-09 - // Not sure about intended relationship between filtering on geometry and - // requesting no geometry? Have preserved current logic of ignoring spatial filter - // if not requesting geometry. - else if ( request.filterType() == QgsFeatureRequest::FilterRect && hasGeometry ) + if ( !request.filterRect().isNull() && hasGeometry ) { QgsDebugMsg( "Configuring for rectangle select" ); mTestGeometry = true; @@ -98,13 +85,32 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe } } - // If we have a subset index then use it.. - if ( mMode == FileScan && mSource->mUseSubsetIndex ) + if ( request.filterType() == QgsFeatureRequest::FilterFid ) { - QgsDebugMsg( QString( "Layer has subset index - use %1 items from subset index" ).arg( mSource->mSubsetIndex.size() ) ); + QgsDebugMsg( "Configuring for returning single id" ); + if ( request.filterRect().isNull() || ( !request.filterRect().isNull() && mFeatureIds.contains( request.filterFid() ) ) ) + { + mFeatureIds = QList() << request.filterFid(); + } + mMode = FeatureIds; mTestSubset = false; - mMode = SubsetIndex; } + // If have geometry and testing geometry then evaluate options... + // If we don't have geometry then all records pass geometry filter. + // CC: 2013-05-09 + // Not sure about intended relationship between filtering on geometry and + // requesting no geometry? Have preserved current logic of ignoring spatial filter + // if not requesting geometry. + + else + + // If we have a subset index then use it.. + if ( mMode == FileScan && mSource->mUseSubsetIndex ) + { + QgsDebugMsg( QString( "Layer has subset index - use %1 items from subset index" ).arg( mSource->mSubsetIndex.size() ) ); + mTestSubset = false; + mMode = SubsetIndex; + } // Otherwise just have to scan the file if ( mMode == FileScan ) diff --git a/src/providers/gpx/qgsgpxfeatureiterator.cpp b/src/providers/gpx/qgsgpxfeatureiterator.cpp index 0ca944e6335..e25d7b30ee6 100644 --- a/src/providers/gpx/qgsgpxfeatureiterator.cpp +++ b/src/providers/gpx/qgsgpxfeatureiterator.cpp @@ -176,7 +176,7 @@ bool QgsGPXFeatureIterator::readFid( QgsFeature& feature ) bool QgsGPXFeatureIterator::readWaypoint( const QgsWaypoint& wpt, QgsFeature& feature ) { - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) + if ( !mRequest.filterRect().isNull() ) { const QgsRectangle& rect = mRequest.filterRect(); if ( ! rect.contains( QgsPoint( wpt.lon, wpt.lat ) ) ) @@ -206,7 +206,7 @@ bool QgsGPXFeatureIterator::readRoute( const QgsRoute& rte, QgsFeature& feature QgsGeometry* theGeometry = readRouteGeometry( rte ); - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) + if ( !mRequest.filterRect().isNull() ) { const QgsRectangle& rect = mRequest.filterRect(); if (( rte.xMax < rect.xMinimum() ) || ( rte.xMin > rect.xMaximum() ) || @@ -248,7 +248,7 @@ bool QgsGPXFeatureIterator::readTrack( const QgsTrack& trk, QgsFeature& feature QgsGeometry* theGeometry = readTrackGeometry( trk ); - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) + if ( !mRequest.filterRect().isNull() ) { const QgsRectangle& rect = mRequest.filterRect(); if (( trk.xMax < rect.xMinimum() ) || ( trk.xMin > rect.xMaximum() ) || diff --git a/src/providers/grass/qgsgrassfeatureiterator.cpp b/src/providers/grass/qgsgrassfeatureiterator.cpp index 1a529b8b187..0d45b13d5c7 100644 --- a/src/providers/grass/qgsgrassfeatureiterator.cpp +++ b/src/providers/grass/qgsgrassfeatureiterator.cpp @@ -80,7 +80,7 @@ QgsGrassFeatureIterator::QgsGrassFeatureIterator( QgsGrassFeatureSource* source, allocateSelection( mSource->mMap ); resetSelection( 1 ); - if ( request.filterType() == QgsFeatureRequest::FilterRect ) + if ( !request.filterRect().isNull() ) { setSelectionRect( request.filterRect(), request.flags() & QgsFeatureRequest::ExactIntersect ); } diff --git a/src/providers/memory/qgsmemoryfeatureiterator.cpp b/src/providers/memory/qgsmemoryfeatureiterator.cpp index 861040f56f7..05aa11c81b1 100644 --- a/src/providers/memory/qgsmemoryfeatureiterator.cpp +++ b/src/providers/memory/qgsmemoryfeatureiterator.cpp @@ -33,14 +33,14 @@ QgsMemoryFeatureIterator::QgsMemoryFeatureIterator( QgsMemoryFeatureSource* sour mSubsetExpression->prepare( mSource->mFields ); } - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect && mRequest.flags() & QgsFeatureRequest::ExactIntersect ) + if ( !mRequest.filterRect().isNull() && mRequest.flags() & QgsFeatureRequest::ExactIntersect ) { mSelectRectGeom = QgsGeometry::fromRect( request.filterRect() ); } // if there's spatial index, use it! // (but don't use it when selection rect is not specified) - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect && mSource->mSpatialIndex ) + if ( !mRequest.filterRect().isNull() && mSource->mSpatialIndex ) { mUsingFeatureIdList = true; mFeatureIdList = mSource->mSpatialIndex->intersects( mRequest.filterRect() ); @@ -90,7 +90,7 @@ bool QgsMemoryFeatureIterator::nextFeatureUsingList( QgsFeature& feature ) // option 1: we have a list of features to traverse while ( mFeatureIdListIterator != mFeatureIdList.constEnd() ) { - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect && mRequest.flags() & QgsFeatureRequest::ExactIntersect ) + if ( !mRequest.filterRect().isNull() && mRequest.flags() & QgsFeatureRequest::ExactIntersect ) { // do exact check in case we're doing intersection if ( mSource->mFeatures[*mFeatureIdListIterator].geometry() && mSource->mFeatures[*mFeatureIdListIterator].geometry()->intersects( mSelectRectGeom ) ) @@ -131,7 +131,7 @@ bool QgsMemoryFeatureIterator::nextFeatureTraverseAll( QgsFeature& feature ) // option 2: traversing the whole layer while ( mSelectIterator != mSource->mFeatures.constEnd() ) { - if ( mRequest.filterType() != QgsFeatureRequest::FilterRect ) + if ( mRequest.filterRect().isNull() ) { // selection rect empty => using all features hasFeature = true; diff --git a/src/providers/mssql/qgsmssqlfeatureiterator.cpp b/src/providers/mssql/qgsmssqlfeatureiterator.cpp index 64e6138c948..1fefff408ac 100644 --- a/src/providers/mssql/qgsmssqlfeatureiterator.cpp +++ b/src/providers/mssql/qgsmssqlfeatureiterator.cpp @@ -89,7 +89,7 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request ) bool filterAdded = false; // set spatial filter - if ( request.filterType() == QgsFeatureRequest::FilterRect && mSource->isSpatial() && !request.filterRect().isEmpty() ) + if ( !request.filterRect().isNull() && mSource->isSpatial() && !request.filterRect().isEmpty() ) { // polygons should be CCW for SqlGeography QString r; diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index b02964bc7df..e0326eeafb0 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -58,20 +58,20 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool mSubsetStringSet = true; } - mFetchGeometry = ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) || !( mRequest.flags() & QgsFeatureRequest::NoGeometry ); + mFetchGeometry = ( !mRequest.filterRect().isNull() ) || !( mRequest.flags() & QgsFeatureRequest::NoGeometry ); QgsAttributeList attrs = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList(); // 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 // filter if we choose to ignore them (fixes #11223) - if (( mSource->mDriverName != "VRT" && mSource->mDriverName != "OGR_VRT" ) || mRequest.filterType() != QgsFeatureRequest::FilterRect ) + if (( mSource->mDriverName != "VRT" && mSource->mDriverName != "OGR_VRT" ) || mRequest.filterRect().isNull() ) { QgsOgrUtils::setRelevantFields( ogrLayer, mSource->mFields.count(), mFetchGeometry, attrs ); } // spatial query to select features - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) + if ( !mRequest.filterRect().isNull() ) { const QgsRectangle& rect = mRequest.filterRect(); @@ -179,7 +179,7 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature ) if ( !readFeature( fet, feature ) ) continue; - if ( mRequest.filterType() == QgsFeatureRequest::FilterRect && !feature.constGeometry() ) + if ( !mRequest.filterRect().isNull() && !feature.constGeometry() ) continue; // we have a feature, end this cycle diff --git a/src/providers/oracle/qgsoraclefeatureiterator.cpp b/src/providers/oracle/qgsoraclefeatureiterator.cpp index ef617fcf2af..9fdd8bc10a1 100644 --- a/src/providers/oracle/qgsoraclefeatureiterator.cpp +++ b/src/providers/oracle/qgsoraclefeatureiterator.cpp @@ -46,45 +46,49 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour QString whereClause; + if ( !mRequest.filterRect().isNull && !mSource->mGeometryColumn.isNull() && mSource->mHasSpatialIndex ) + { + QgsRectangle rect( mRequest.filterRect() ); + QString bbox = QString( "mdsys.sdo_geometry(2003,%1,NULL," + "mdsys.sdo_elem_info_array(1,1003,3)," + "mdsys.sdo_ordinate_array(%2,%3,%4,%5)" + ")" ) + .arg( mSource->mSrid < 1 ? "NULL" : QString::number( mSource->mSrid ) ) + .arg( qgsDoubleToString( rect.xMinimum() ) ) + .arg( qgsDoubleToString( rect.yMinimum() ) ) + .arg( qgsDoubleToString( rect.xMaximum() ) ) + .arg( qgsDoubleToString( rect.yMaximum() ) ); + + whereClause = QString( "sdo_filter(%1,%2)='TRUE'" ).arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ).arg( bbox ); + + if ( mRequest.flags() & QgsFeatureRequest::ExactIntersect && mConnection->hasSpatial() ) + { + whereClause += QString( " AND sdo_relate(%1,%2,'mask=ANYINTERACT')='TRUE'" ) + .arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ) + .arg( bbox ); + } + } + switch ( request.filterType() ) { + case QgsFeatureRequest::FilterFid: + QString fidWhereClause = QgsOracleUtils::whereClause( request.filterFid(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + whereClause = QgsPostgresUtils::andWhereClauses( whereClause, fidWhereClause ); + break; + + case QgsFeatureRequest::FilterFids: + QString fidsWhereClause = QgsOracleUtils::whereClause( request.filterFids(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + whereClause = QgsPostgresUtils::andWhereClauses( whereClause, fidsWhereClause ); + break; + + case QgsFeatureRequest::FilterNone: + break; + case QgsFeatureRequest::FilterExpression: break; case QgsFeatureRequest::FilterRect: - if ( !mSource->mGeometryColumn.isNull() && mSource->mHasSpatialIndex ) - { - QgsRectangle rect( mRequest.filterRect() ); - QString bbox = QString( "mdsys.sdo_geometry(2003,%1,NULL," - "mdsys.sdo_elem_info_array(1,1003,3)," - "mdsys.sdo_ordinate_array(%2,%3,%4,%5)" - ")" ) - .arg( mSource->mSrid < 1 ? "NULL" : QString::number( mSource->mSrid ) ) - .arg( qgsDoubleToString( rect.xMinimum() ) ) - .arg( qgsDoubleToString( rect.yMinimum() ) ) - .arg( qgsDoubleToString( rect.xMaximum() ) ) - .arg( qgsDoubleToString( rect.yMaximum() ) ); - - whereClause = QString( "sdo_filter(%1,%2)='TRUE'" ).arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ).arg( bbox ); - - if ( mRequest.flags() & QgsFeatureRequest::ExactIntersect && mConnection->hasSpatial() ) - { - whereClause += QString( " AND sdo_relate(%1,%2,'mask=ANYINTERACT')='TRUE'" ) - .arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ) - .arg( bbox ); - } - } - break; - - case QgsFeatureRequest::FilterFid: - whereClause = QgsOracleUtils::whereClause( request.filterFid(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); - break; - - case QgsFeatureRequest::FilterFids: - whereClause = QgsOracleUtils::whereClause( request.filterFids(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); - break; - - case QgsFeatureRequest::FilterNone: + // Handled in the if-statement above break; } diff --git a/src/providers/oracle/qgsoracleprovider.cpp b/src/providers/oracle/qgsoracleprovider.cpp index 0e1d0cc941e..80674408cd6 100644 --- a/src/providers/oracle/qgsoracleprovider.cpp +++ b/src/providers/oracle/qgsoracleprovider.cpp @@ -464,6 +464,15 @@ QString QgsOracleUtils::whereClause( QgsFeatureIds featureIds, const QgsFields & return whereClauses.isEmpty() ? "" : whereClauses.join( " OR " ).prepend( "(" ).append( ")" ); } +QString QgsOracleUtils::andWhereClauses( const QString& c1, const QString& c2 ) +{ + if ( c1.isNull() ) + return c2; + if ( c2.isNull() ) + return c1; + + return QString( "(%1) AND (%2)" ).arg( c1 ).arg( c2 ); +} QString QgsOracleProvider::whereClause( QgsFeatureId featureId ) const { diff --git a/src/providers/oracle/qgsoracleprovider.h b/src/providers/oracle/qgsoracleprovider.h index 6e389ae9ee5..585eca20f17 100644 --- a/src/providers/oracle/qgsoracleprovider.h +++ b/src/providers/oracle/qgsoracleprovider.h @@ -423,6 +423,8 @@ class QgsOracleUtils QgsOraclePrimaryKeyType primaryKeyType, const QList& primaryKeyAttrs, QSharedPointer sharedData ); + + static QString andWhereClauses( const QString& c1, const QString& c2 ); }; diff --git a/src/providers/postgres/qgspostgresexpressioncompiler.cpp b/src/providers/postgres/qgspostgresexpressioncompiler.cpp index f281ed26439..f09e5b8c35a 100644 --- a/src/providers/postgres/qgspostgresexpressioncompiler.cpp +++ b/src/providers/postgres/qgspostgresexpressioncompiler.cpp @@ -184,9 +184,32 @@ QgsPostgresExpressionCompiler::Result QgsPostgresExpressionCompiler::compile( co return Complete; } + case QgsExpression::ntInOperator: + { + const QgsExpression::NodeInOperator* n = static_cast( node ); + QStringList list; + + Q_FOREACH ( const QgsExpression::Node* ln, n->list()->list() ) + { + QString s; + Result r = compile( ln, s ); + if ( r == Complete ) + list << s; + else + return r; + } + + QString nd; + Result rn = compile( n->node(), nd ); + if ( rn != Complete ) + return rn; + + result = QString( "%1 %2IN(%3)" ).arg( nd ).arg( n->isNotIn() ? "NOT " : "" ).arg( list.join( "," ) ); + return Complete; + } + case QgsExpression::ntFunction: case QgsExpression::ntCondition: - case QgsExpression::ntInOperator: break; } diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp index 11c2e6be6b6..4274e342c3b 100644 --- a/src/providers/postgres/qgspostgresfeatureiterator.cpp +++ b/src/providers/postgres/qgspostgresfeatureiterator.cpp @@ -58,17 +58,22 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource mCursorName = mConn->uniqueCursorName(); QString whereClause; - if ( request.filterType() == QgsFeatureRequest::FilterRect && !mSource->mGeometryColumn.isNull() ) + if ( !request.filterRect().isNull() && !mSource->mGeometryColumn.isNull() ) { whereClause = whereClauseRect(); } - else if ( request.filterType() == QgsFeatureRequest::FilterFid ) + + if ( request.filterType() == QgsFeatureRequest::FilterFid ) { - whereClause = QgsPostgresUtils::whereClause( mRequest.filterFid(), mSource->mFields, mConn, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + QString fidWhereClause = QgsPostgresUtils::whereClause( mRequest.filterFid(), mSource->mFields, mConn, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + + whereClause = QgsPostgresUtils::andWhereClauses( whereClause, fidWhereClause ); } else if ( request.filterType() == QgsFeatureRequest::FilterFids ) { - whereClause = QgsPostgresUtils::whereClause( mRequest.filterFids(), mSource->mFields, mConn, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + QString fidsWhereClause = QgsPostgresUtils::whereClause( mRequest.filterFids(), mSource->mFields, mConn, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared ); + + whereClause = QgsPostgresUtils::andWhereClauses( whereClause, fidsWhereClause ); } else if ( request.filterType() == QgsFeatureRequest::FilterExpression && QSettings().value( "/qgis/postgres/compileExpressions", false ).toBool() ) @@ -77,7 +82,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource if ( compiler.compile( request.filterExpression() ) == QgsPostgresExpressionCompiler::Complete ) { - whereClause = compiler.result(); + whereClause = QgsPostgresUtils::andWhereClauses( whereClause, compiler.result() ); mExpressionCompiled = true; } } diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 4fd4ac73ed4..444c618cfa2 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -548,6 +548,16 @@ QString QgsPostgresUtils::whereClause( QgsFeatureIds featureIds, const QgsFields return whereClauses.isEmpty() ? "" : whereClauses.join( " OR " ).prepend( "(" ).append( ")" ); } +QString QgsPostgresUtils::andWhereClauses( const QString& c1, const QString& c2 ) +{ + if ( c1.isNull() ) + return c2; + if ( c2.isNull() ) + return c1; + + return QString( "(%1) AND (%2)" ).arg( c1 ).arg( c2 ); +} + QString QgsPostgresProvider::filterWhereClause() const { QString where; diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h index 33316961942..261bdecf91b 100644 --- a/src/providers/postgres/qgspostgresprovider.h +++ b/src/providers/postgres/qgspostgresprovider.h @@ -501,6 +501,8 @@ class QgsPostgresUtils QgsPostgresPrimaryKeyType pkType, const QList& pkAttrs, QSharedPointer sharedData ); + + static QString andWhereClauses( const QString& c1, const QString& c2 ); }; /** Data shared between provider class and its feature sources. Ideally there should diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp index 122c4a3eb3c..67f6cf1ba37 100644 --- a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp +++ b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp @@ -35,7 +35,7 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature mRowNumber = 0; QString whereClause; - if ( request.filterType() == QgsFeatureRequest::FilterRect && !mSource->mGeometryColumn.isNull() ) + if ( !request.filterRect().isNull() && !mSource->mGeometryColumn.isNull() ) { // some kind of MBR spatial filtering is required whereClause += whereClauseRect(); @@ -45,8 +45,7 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature { whereClause += whereClauseFid(); } - - if ( request.filterType() == QgsFeatureRequest::FilterFids ) + else if ( request.filterType() == QgsFeatureRequest::FilterFids ) { whereClause += whereClauseFids(); } diff --git a/src/providers/wfs/qgswfsfeatureiterator.cpp b/src/providers/wfs/qgswfsfeatureiterator.cpp index 46f0bca800c..45294aeae25 100644 --- a/src/providers/wfs/qgswfsfeatureiterator.cpp +++ b/src/providers/wfs/qgswfsfeatureiterator.cpp @@ -21,20 +21,18 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource* source, bool ownSource, const QgsFeatureRequest& request ) : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) { - switch ( request.filterType() ) + if ( !request.filterRect().isNull() && mSource->mSpatialIndex ) { - case QgsFeatureRequest::FilterRect: - if ( mSource->mSpatialIndex ) - { - mSelectedFeatures = mSource->mSpatialIndex->intersects( request.filterRect() ); - } - break; - case QgsFeatureRequest::FilterFid: - mSelectedFeatures.push_back( request.filterFid() ); - break; - case QgsFeatureRequest::FilterNone: - default: - mSelectedFeatures = mSource->mFeatures.keys(); + mSelectedFeatures = mSource->mSpatialIndex->intersects( request.filterRect() ); + } + + if ( request.filterType() == QgsFeatureRequest::FilterFid ) + { + mSelectedFeatures.push_back( request.filterFid() ); + } + else + { + mSelectedFeatures = mSource->mFeatures.keys(); } mFeatureIterator = mSelectedFeatures.constBegin(); @@ -164,7 +162,7 @@ QgsWFSFeatureSource::~QgsWFSFeatureSource() QgsFeatureIterator QgsWFSFeatureSource::getFeatures( const QgsFeatureRequest& request ) { - if ( request.filterType() == QgsFeatureRequest::FilterRect ) + if ( !request.filterRect().isNull() ) emit extentRequested( request.filterRect() ); return QgsFeatureIterator( new QgsWFSFeatureIterator( this, false, request ) ); } diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py index 04040a617f7..e21fb6230d9 100644 --- a/tests/src/python/providertestbase.py +++ b/tests/src/python/providertestbase.py @@ -72,6 +72,15 @@ class ProviderTestCase(object): features = [f['pk'] for f in self.provider.getFeatures(QgsFeatureRequest().setFilterRect(extent))] assert set(features) == set([2L, 4L]), 'Got {} instead'.format(features) + def testRectAndExpression(self): + extent = QgsRectangle(-70, 67, -60, 80) + result = set([f['pk'] for f in self.provider.getFeatures( + QgsFeatureRequest() + .setFilterExpression('"cnt">200') + .setFilterRect(extent))]) + expected=[4L] + assert set(expected) == result, 'Expected {} and got {} when testing for combination of filterRect and expression'.format(set(expected), result) + def testMinValue(self): assert self.provider.minimumValue(1) == -200 assert self.provider.minimumValue(2) == 'Apple'