Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[ogr] Fix very slow feature requests when filter string set
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
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
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.