Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix failing test
We need to ensure that the FID column is also fetched for the
unfiltered layer which we use when requesting features by ID
  • Loading branch information
nyalldawson committed Jun 2, 2018
1 parent f3bc283 commit ca1262d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
19 changes: 15 additions & 4 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -66,7 +66,8 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
if ( !mSource->mSubsetString.isEmpty() )
{
mOgrOrigLayer = mOgrLayer;
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrOrigLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, mOrigFidAdded );
mOgrLayerWithFid = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, QString(), mOrigFidAdded );
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, mOrigFidAdded );
if ( !mOgrLayer )
{
close();
Expand Down Expand Up @@ -94,6 +95,8 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
( mSource->mOgrGeometryTypeFilter != wkbUnknown );

QgsAttributeList attrs = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList();
if ( mOrigFidAdded )
attrs << mSource->mFields.count();

// ensure that all attributes required for expression filter are being fetched
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes && request.filterType() == QgsFeatureRequest::FilterExpression )
Expand Down Expand Up @@ -128,17 +131,23 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
// filter if we choose to ignore them (fixes #11223)
if ( ( mSource->mDriverName != QLatin1String( "VRT" ) && mSource->mDriverName != QLatin1String( "OGR_VRT" ) ) || mFilterRect.isNull() )
{
QgsOgrProviderUtils::setRelevantFields( mOgrLayer, mSource->mFields.count(), mFetchGeometry, attrs, mSource->mFirstFieldIsFid );
QgsOgrProviderUtils::setRelevantFields( mOgrLayer, mSource->mFields.count() + ( mOrigFidAdded ? 1 : 0 ), mFetchGeometry, attrs, mSource->mFirstFieldIsFid );
if ( mOgrLayerWithFid )
QgsOgrProviderUtils::setRelevantFields( mOgrLayerWithFid, mSource->mFields.count() + ( mOrigFidAdded ? 1 : 0 ), mFetchGeometry, attrs, mSource->mFirstFieldIsFid );
}

// spatial query to select features
if ( !mFilterRect.isNull() )
{
OGR_L_SetSpatialFilterRect( mOgrLayer, mFilterRect.xMinimum(), mFilterRect.yMinimum(), mFilterRect.xMaximum(), mFilterRect.yMaximum() );
if ( mOgrLayerWithFid )
OGR_L_SetSpatialFilterRect( mOgrLayerWithFid, mFilterRect.xMinimum(), mFilterRect.yMinimum(), mFilterRect.xMaximum(), mFilterRect.yMaximum() );
}
else
{
OGR_L_SetSpatialFilter( mOgrLayer, nullptr );
if ( mOgrLayerWithFid )
OGR_L_SetSpatialFilter( mOgrLayerWithFid, nullptr );
}

if ( request.filterType() == QgsFeatureRequest::FilterExpression
Expand Down Expand Up @@ -205,8 +214,8 @@ bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature &fea
// string in place), then we need to request features from the original, unfiltered
// OGR layer. Otherwise the feature IDs we request will correspond to features in the
// filtered layer, not the original layer!
OGR_L_ResetReading( mOgrOrigLayer );
fet.reset( OGR_L_GetFeature( mOgrOrigLayer, FID_TO_NUMBER( id ) ) );
OGR_L_ResetReading( mOgrLayerWithFid );
fet.reset( OGR_L_GetFeature( mOgrLayerWithFid, FID_TO_NUMBER( id ) ) );
}
else
{
Expand Down Expand Up @@ -303,8 +312,10 @@ bool QgsOgrFeatureIterator::close()
if ( mOgrOrigLayer )
{
GDALDatasetReleaseResultSet( mConn->ds, mOgrLayer );
GDALDatasetReleaseResultSet( mConn->ds, mOgrLayerWithFid );
mOgrLayer = mOgrOrigLayer;
mOgrOrigLayer = nullptr;
mOgrLayerWithFid = nullptr;
}

if ( mConn )
Expand Down
1 change: 1 addition & 0 deletions src/providers/ogr/qgsogrfeatureiterator.h
Expand Up @@ -74,6 +74,7 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
QgsOgrConn *mConn = nullptr;
OGRLayerH mOgrLayer = nullptr;
OGRLayerH mOgrOrigLayer = nullptr;
OGRLayerH mOgrLayerWithFid = nullptr;

bool mOrigFidAdded = false;

Expand Down
5 changes: 3 additions & 2 deletions src/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -3919,8 +3919,9 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
else
{
QByteArray sqlPart1 = "SELECT *";
QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, mGDALDriverName )
+ " WHERE " + encoding->fromUnicode( subsetString );
QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, mGDALDriverName );
if ( !subsetString.isEmpty() )
sqlPart3 += " WHERE " + encoding->fromUnicode( subsetString );

origFidAddAttempted = true;

Expand Down

0 comments on commit ca1262d

Please sign in to comment.