Skip to content

Commit

Permalink
[ogr] Fix very slow feature requests when filter string set
Browse files Browse the repository at this point in the history
Follow up 217e700. Avoid the very expensive iteration to
find matching features when a subset string is set by
instead querying the original, unfiltered layer when
we are doing a FilterFids type request.

Fixes many hangs when using OGR layers with filters in place.
  • Loading branch information
nyalldawson committed Jun 2, 2018
1 parent bab8b78 commit 2975339
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
35 changes: 23 additions & 12 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool

if ( !mSource->mSubsetString.isEmpty() )
{
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, mOrigFidAdded );
mOgrOrigLayer = mOgrLayer;
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrOrigLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, mOrigFidAdded );
if ( !mOgrLayer )
{
close();
return;
}
mSubsetStringSet = true;
}

if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs )
Expand Down Expand Up @@ -200,18 +201,26 @@ bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature &fea
gdal::ogr_feature_unique_ptr fet;
if ( mOrigFidAdded )
{
OGR_L_ResetReading( mOgrLayer );
OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( mOgrLayer );
// if we have been using the original fid as feature IDs (e.g. as a result of a subset
// 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 ) ) );

// do a bit of a safety check - make sure that the original fid column value
// matches the feature id which we actually requested.
OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( mOgrOrigLayer );
int lastField = OGR_FD_GetFieldCount( fdef ) - 1;
bool foundCorrect = false;
if ( lastField >= 0 )
foundCorrect = OGR_F_GetFieldAsInteger64( fet.get(), lastField ) == id;
else
foundCorrect = OGR_F_GetFID( fet.get() ) == id;

if ( !foundCorrect )
{
while ( fet.reset( OGR_L_GetNextFeature( mOgrLayer ) ), fet )
{
if ( OGR_F_GetFieldAsInteger64( fet.get(), lastField ) == id )
{
break;
}
}
fet.reset();
}
}
else
Expand Down Expand Up @@ -306,9 +315,11 @@ bool QgsOgrFeatureIterator::close()
OGR_L_ResetReading( mOgrLayer );
}

if ( mSubsetStringSet )
if ( mOgrOrigLayer )
{
GDALDatasetReleaseResultSet( mConn->ds, mOgrLayer );
mOgrLayer = mOgrOrigLayer;
mOgrOrigLayer = nullptr;
}

if ( mConn )
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ogr/qgsogrfeatureiterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr

QgsOgrConn *mConn = nullptr;
OGRLayerH mOgrLayer = nullptr;
OGRLayerH mOgrOrigLayer = nullptr;

bool mSubsetStringSet = false;
bool mOrigFidAdded = false;

//! Sets to true, if geometry is in the requested columns
Expand Down

0 comments on commit 2975339

Please sign in to comment.