From 0b9d1dc49b3277a842d0a18a2f1ba99782965999 Mon Sep 17 00:00:00 2001 From: Nathan Woodrow Date: Wed, 19 Jun 2013 22:31:50 +1000 Subject: [PATCH 1/4] Use a new ogr layer per iterator --- src/providers/ogr/qgsogrfeatureiterator.cpp | 31 ++++++++++++--------- src/providers/ogr/qgsogrfeatureiterator.h | 4 +++ src/providers/ogr/qgsogrprovider.cpp | 3 +- src/providers/ogr/qgsogrprovider.h | 6 ++++ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index dd1cf4ddfd96..ebe9b323207b 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -32,15 +32,19 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatureRequest& request ) - : QgsAbstractFeatureIterator( request ), P( p ) + : QgsAbstractFeatureIterator( request ), P( p ), ogrDataSource(0), ogrLayer(0), ogrDriver(0) { - // make sure that only one iterator is active - if ( P->mActiveIterator ) + + ogrDataSource = OGROpen( TO8F( P->filePath() ), false, &ogrDriver ); + + if ( P->layerName().isNull() ) + { + ogrLayer = OGR_DS_GetLayer( ogrDataSource, P->layerIndex() ); + } + else { - QgsMessageLog::logMessage( QObject::tr( "Already active iterator on this provider was closed." ), QObject::tr( "OGR" ) ); - P->mActiveIterator->close(); + ogrLayer = OGR_DS_GetLayerByName( ogrDataSource, TO8( p->layerName() ) ); } - P->mActiveIterator = this; mFeatureFetched = false; @@ -56,12 +60,12 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatur OGR_G_CreateFromWkt(( char ** )&wktText, NULL, &filter ); QgsDebugMsg( "Setting spatial filter using " + wktExtent ); - OGR_L_SetSpatialFilter( P->ogrLayer, filter ); + OGR_L_SetSpatialFilter( ogrLayer, filter ); OGR_G_DestroyGeometry( filter ); } else { - OGR_L_SetSpatialFilter( P->ogrLayer, 0 ); + OGR_L_SetSpatialFilter( ogrLayer, 0 ); } //start with first feature @@ -94,7 +98,7 @@ bool QgsOgrFeatureIterator::nextFeature( QgsFeature& feature ) if ( mRequest.filterType() == QgsFeatureRequest::FilterFid ) { - OGRFeatureH fet = OGR_L_GetFeature( P->ogrLayer, FID_TO_NUMBER( mRequest.filterFid() ) ); + OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( mRequest.filterFid() ) ); if ( !fet ) { close(); @@ -111,7 +115,7 @@ bool QgsOgrFeatureIterator::nextFeature( QgsFeature& feature ) OGRFeatureH fet; - while (( fet = OGR_L_GetNextFeature( P->ogrLayer ) ) ) + while (( fet = OGR_L_GetNextFeature( ogrLayer ) ) ) { if ( !readFeature( fet, feature ) ) continue; @@ -135,7 +139,7 @@ bool QgsOgrFeatureIterator::rewind() if ( mClosed ) return false; - OGR_L_ResetReading( P->ogrLayer ); + OGR_L_ResetReading( ogrLayer ); return true; } @@ -146,10 +150,11 @@ bool QgsOgrFeatureIterator::close() if ( mClosed ) return false; - // tell provider that this iterator is not active anymore - P->mActiveIterator = 0; + OGR_DS_ReleaseResultSet( ogrDataSource, ogrLayer ); + OGR_DS_Destroy( ogrDataSource ); mClosed = true; + ogrDataSource = 0; return true; } diff --git a/src/providers/ogr/qgsogrfeatureiterator.h b/src/providers/ogr/qgsogrfeatureiterator.h index 97d4d7d6dfff..41c6082410b2 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.h +++ b/src/providers/ogr/qgsogrfeatureiterator.h @@ -48,6 +48,10 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIterator void getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature & f, int attindex ); bool mFeatureFetched; + + OGRDataSourceH ogrDataSource; + OGRLayerH ogrLayer; + OGRSFDriverH ogrDriver; }; diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp index cc84a403b8c5..bb9fccac6265 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -360,8 +360,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri ) QgsOgrProvider::~QgsOgrProvider() { - if ( mActiveIterator ) - mActiveIterator->close(); + // Do we need to close all active iterators here? if ( ogrLayer != ogrOrigLayer ) { diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h index e57bab279857..ba406647f56c 100644 --- a/src/providers/ogr/qgsogrprovider.h +++ b/src/providers/ogr/qgsogrprovider.h @@ -250,6 +250,12 @@ class QgsOgrProvider : public QgsVectorDataProvider /** Get single flatten geometry type */ static OGRwkbGeometryType ogrWkbSingleFlatten( OGRwkbGeometryType type ); + QString layerName() { return mLayerName; } + + QString filePath() { return mFilePath; } + + int layerIndex() { return mLayerIndex; } + protected: /** loads fields from input file to member attributeFields */ void loadFields(); From 6eadd80fcbb0f5ba598f4fffbe2b2216863fd0e9 Mon Sep 17 00:00:00 2001 From: Nathan Woodrow Date: Thu, 20 Jun 2013 09:04:22 +1000 Subject: [PATCH 2/4] Remove driver --- src/providers/ogr/qgsogrfeatureiterator.cpp | 3 +-- src/providers/ogr/qgsogrfeatureiterator.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index ebe9b323207b..aeb107cb19e2 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -35,7 +35,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatur : QgsAbstractFeatureIterator( request ), P( p ), ogrDataSource(0), ogrLayer(0), ogrDriver(0) { - ogrDataSource = OGROpen( TO8F( P->filePath() ), false, &ogrDriver ); + ogrDataSource = OGROpen( TO8F( P->filePath() ), false, NULL ); if ( P->layerName().isNull() ) { @@ -150,7 +150,6 @@ bool QgsOgrFeatureIterator::close() if ( mClosed ) return false; - OGR_DS_ReleaseResultSet( ogrDataSource, ogrLayer ); OGR_DS_Destroy( ogrDataSource ); mClosed = true; diff --git a/src/providers/ogr/qgsogrfeatureiterator.h b/src/providers/ogr/qgsogrfeatureiterator.h index 41c6082410b2..2ec9b86a5cd0 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.h +++ b/src/providers/ogr/qgsogrfeatureiterator.h @@ -51,7 +51,6 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIterator OGRDataSourceH ogrDataSource; OGRLayerH ogrLayer; - OGRSFDriverH ogrDriver; }; From 0c88963c65d3292e321578104505e8ecc69a6a68 Mon Sep 17 00:00:00 2001 From: Nathan Woodrow Date: Thu, 20 Jun 2013 22:18:31 +1000 Subject: [PATCH 3/4] Set filter on iterator: --- src/providers/ogr/qgsogrfeatureiterator.cpp | 18 ++++++++++++++++-- src/providers/ogr/qgsogrfeatureiterator.h | 2 ++ src/providers/ogr/qgsogrprovider.h | 8 ++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index aeb107cb19e2..105d8f4ee66e 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -32,8 +32,9 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatureRequest& request ) - : QgsAbstractFeatureIterator( request ), P( p ), ogrDataSource(0), ogrLayer(0), ogrDriver(0) + : QgsAbstractFeatureIterator( request ), P( p ), ogrDataSource(0), ogrLayer(0), mSubsetStringSet(false) { + mFeatureFetched = false; ogrDataSource = OGROpen( TO8F( P->filePath() ), false, NULL ); @@ -46,7 +47,15 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatur ogrLayer = OGR_DS_GetLayerByName( ogrDataSource, TO8( p->layerName() ) ); } - mFeatureFetched = false; + if ( !P->subsetString().isEmpty() ) + { + QString sql = QString( "SELECT * FROM %1 WHERE %2" ) + .arg( P->quotedIdentifier( FROM8( OGR_FD_GetName( OGR_L_GetLayerDefn( ogrLayer ) ) ) ) ) + .arg( P->subsetString() ); + QgsDebugMsg( QString( "SQL: %1" ).arg( sql ) ); + ogrLayer = OGR_DS_ExecuteSQL( ogrDataSource, P->textEncoding()->fromUnicode( sql ).constData(), NULL, NULL ); + mSubsetStringSet = true; + } ensureRelevantFields(); @@ -150,6 +159,11 @@ bool QgsOgrFeatureIterator::close() if ( mClosed ) return false; + if (mSubsetStringSet) + { + OGR_DS_ReleaseResultSet(ogrDataSource, ogrLayer ); + } + OGR_DS_Destroy( ogrDataSource ); mClosed = true; diff --git a/src/providers/ogr/qgsogrfeatureiterator.h b/src/providers/ogr/qgsogrfeatureiterator.h index 2ec9b86a5cd0..da8938d00783 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.h +++ b/src/providers/ogr/qgsogrfeatureiterator.h @@ -51,6 +51,8 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIterator OGRDataSourceH ogrDataSource; OGRLayerH ogrLayer; + + bool mSubsetStringSet; }; diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h index ba406647f56c..216dcb2b806d 100644 --- a/src/providers/ogr/qgsogrprovider.h +++ b/src/providers/ogr/qgsogrprovider.h @@ -15,6 +15,8 @@ email : sherman at mrcc.com * * ***************************************************************************/ +#include "QTextCodec" + #include "qgsrectangle.h" #include "qgsvectordataprovider.h" #include "qgsvectorfilewriter.h" @@ -256,6 +258,10 @@ class QgsOgrProvider : public QgsVectorDataProvider int layerIndex() { return mLayerIndex; } + QTextCodec* textEncoding() { return mEncoding; } + + QString quotedIdentifier( QString field ); + protected: /** loads fields from input file to member attributeFields */ void loadFields(); @@ -329,8 +335,6 @@ class QgsOgrProvider : public QgsVectorDataProvider /**Deletes one feature*/ bool deleteFeature( QgsFeatureId id ); - QString quotedIdentifier( QString field ); - /**Calls OGR_L_SyncToDisk and recreates the spatial index if present*/ bool syncToDisc(); From 6876540302bd0c2e75ca6c97db8a99765fb10f5d Mon Sep 17 00:00:00 2001 From: Nathan Woodrow Date: Fri, 21 Jun 2013 14:27:28 +1000 Subject: [PATCH 4/4] Close active iterators --- src/providers/ogr/qgsogrfeatureiterator.cpp | 3 +++ src/providers/ogr/qgsogrprovider.cpp | 6 +++++- src/providers/ogr/qgsogrprovider.h | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index 105d8f4ee66e..72b0d0376419 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -35,6 +35,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrProvider* p, const QgsFeatur : QgsAbstractFeatureIterator( request ), P( p ), ogrDataSource(0), ogrLayer(0), mSubsetStringSet(false) { mFeatureFetched = false; + P->mActiveIterators.insert( this ); ogrDataSource = OGROpen( TO8F( P->filePath() ), false, NULL ); @@ -159,6 +160,8 @@ bool QgsOgrFeatureIterator::close() if ( mClosed ) return false; + P->mActiveIterators.remove( this ); + if (mSubsetStringSet) { OGR_DS_ReleaseResultSet(ogrDataSource, ogrLayer ); diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp index bb9fccac6265..6bd9b8e1ddf9 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -209,7 +209,6 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri ) , ogrDriver( 0 ) , valid( false ) , featuresCounted( -1 ) - , mActiveIterator( 0 ) { QgsCPLErrorHandler handler; @@ -362,6 +361,11 @@ QgsOgrProvider::~QgsOgrProvider() { // Do we need to close all active iterators here? + foreach ( QgsOgrFeatureIterator* it, mActiveIterators ) + { + it->close(); + } + if ( ogrLayer != ogrOrigLayer ) { OGR_DS_ReleaseResultSet( ogrDataSource, ogrLayer ); diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h index 216dcb2b806d..837f0a91e3f7 100644 --- a/src/providers/ogr/qgsogrprovider.h +++ b/src/providers/ogr/qgsogrprovider.h @@ -339,5 +339,5 @@ class QgsOgrProvider : public QgsVectorDataProvider bool syncToDisc(); friend class QgsOgrFeatureIterator; - QgsOgrFeatureIterator* mActiveIterator; //!< pointer to currently active iterator (0 if none) + QSet< QgsOgrFeatureIterator*> mActiveIterators; };