Postgres feature iterator fixes (rewind, FID filter, closing)

wonder-sk · wonder-sk · commit 353e7ab8cd7e · 2013-01-06T21:53:58.000+01:00
diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -32,7 +32,6 @@ const int QgsPostgresFeatureIterator::sFeatureQueueSize = 2000;
 QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresProvider* p, const QgsFeatureRequest& request )
     : QgsAbstractFeatureIterator( request ), P( p )
     , mFeatureQueueSize( sFeatureQueueSize )
-    , mUseQueue( true )
 {
   mCursorName = QString( "qgisf%1" ).arg( P->mProviderId );
 
@@ -45,7 +44,6 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresProvider* p,
   else if ( request.filterType() == QgsFeatureRequest::FilterFid )
   {
     whereClause = P->whereClause( request.filterFid() );
-    mUseQueue = false; // queue not necessary
   }
 
   if ( !P->mSqlWhereClause.isEmpty() )
@@ -79,12 +77,12 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )
   if ( mClosed )
     return false;
 
+#if 0
+  // featureAtId used to have some special checks - necessary?
   if ( !mUseQueue )
   {
     QgsPostgresResult queryResult = P->mConnectionRO->PQexec( QString( "FETCH FORWARD 1 FROM %1" ).arg( mCursorName ) );
 
-    /*
-    // TODO: really needed?
     int rows = queryResult.PQntuples();
     if ( rows == 0 )
     {
@@ -95,14 +93,15 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )
     else if ( rows != 1 )
     {
       QgsMessageLog::logMessage( tr( "found %1 features instead of just one." ).arg( rows ), tr( "PostGIS" ) );
-    }*/
+    }
 
     bool gotit = getFeature( queryResult, 0, feature );
 
     feature.setValid( gotit );
     feature.setFields( &P->mAttributeFields ); // allow name-based attribute lookups
     return gotit;
   }
+#endif
 
   if ( mFeatureQueue.empty() )
   {
@@ -141,8 +140,7 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )
   if ( mFeatureQueue.empty() )
   {
     QgsDebugMsg( QString( "Finished after %1 features" ).arg( mFetched ) );
-    P->mConnectionRO->closeCursor( mCursorName );
-    mClosed = true;
+
     if ( P->mFeaturesCounted < mFetched )
     {
       QgsDebugMsg( QString( "feature count adjusted from %1 to %2" ).arg( P->mFeaturesCounted ).arg( mFetched ) );
@@ -179,8 +177,9 @@ bool QgsPostgresFeatureIterator::rewind()
     return false;
 
   // move cursor to first record
-  P->mConnectionRO->PQexecNR( QString( "move 0 in %1" ).arg( mCursorName ) );
+  P->mConnectionRO->PQexecNR( QString( "move absolute 0 in %1" ).arg( mCursorName ) );
   mFeatureQueue.empty();
+  mFetched = 0;
 
   return true;
 }
@@ -197,6 +196,7 @@ bool QgsPostgresFeatureIterator::close()
     mFeatureQueue.dequeue();
   }
 
+  mClosed = true;
   return true;
 }
 
diff --git a/src/providers/postgres/qgspostgresfeatureiterator.h b/src/providers/postgres/qgspostgresfeatureiterator.h
@@ -49,8 +49,6 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIterator
 
     static const int sFeatureQueueSize;
 
-    //! Whether to use mFeatureQueue
-    bool mUseQueue;
 };
 
 #endif // QGSPOSTGRESFEATUREITERATOR_H

Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,6 @@ const int QgsPostgresFeatureIterator::sFeatureQueueSize = 2000;`
`32`	`32`	`QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresProvider* p, const QgsFeatureRequest& request )`
`33`	`33`	`: QgsAbstractFeatureIterator( request ), P( p )`
`34`	`34`	`, mFeatureQueueSize( sFeatureQueueSize )`
`35`		`- , mUseQueue( true )`
`36`	`35`	`{`
`37`	`36`	`mCursorName = QString( "qgisf%1" ).arg( P->mProviderId );`
`38`	`37`
`@@ -45,7 +44,6 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresProvider* p,`
`45`	`44`	`else if ( request.filterType() == QgsFeatureRequest::FilterFid )`
`46`	`45`	`{`
`47`	`46`	`whereClause = P->whereClause( request.filterFid() );`
`48`		`- mUseQueue = false; // queue not necessary`
`49`	`47`	`}`
`50`	`48`
`51`	`49`	`if ( !P->mSqlWhereClause.isEmpty() )`
`@@ -79,12 +77,12 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )`
`79`	`77`	`if ( mClosed )`
`80`	`78`	`return false;`
`81`	`79`
	`80`	`+#if 0`
	`81`	`+ // featureAtId used to have some special checks - necessary?`
`82`	`82`	`if ( !mUseQueue )`
`83`	`83`	`{`
`84`	`84`	`QgsPostgresResult queryResult = P->mConnectionRO->PQexec( QString( "FETCH FORWARD 1 FROM %1" ).arg( mCursorName ) );`
`85`	`85`
`86`		`- /*`
`87`		`- // TODO: really needed?`
`88`	`86`	`int rows = queryResult.PQntuples();`
`89`	`87`	`if ( rows == 0 )`
`90`	`88`	`{`
`@@ -95,14 +93,15 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )`
`95`	`93`	`else if ( rows != 1 )`
`96`	`94`	`{`
`97`	`95`	`QgsMessageLog::logMessage( tr( "found %1 features instead of just one." ).arg( rows ), tr( "PostGIS" ) );`
`98`		`- }*/`
	`96`	`+ }`
`99`	`97`
`100`	`98`	`bool gotit = getFeature( queryResult, 0, feature );`
`101`	`99`
`102`	`100`	`feature.setValid( gotit );`
`103`	`101`	`feature.setFields( &P->mAttributeFields ); // allow name-based attribute lookups`
`104`	`102`	`return gotit;`
`105`	`103`	`}`
	`104`	`+#endif`
`106`	`105`
`107`	`106`	`if ( mFeatureQueue.empty() )`
`108`	`107`	`{`
`@@ -141,8 +140,7 @@ bool QgsPostgresFeatureIterator::nextFeature( QgsFeature& feature )`
`141`	`140`	`if ( mFeatureQueue.empty() )`
`142`	`141`	`{`
`143`	`142`	`QgsDebugMsg( QString( "Finished after %1 features" ).arg( mFetched ) );`
`144`		`- P->mConnectionRO->closeCursor( mCursorName );`
`145`		`- mClosed = true;`
	`143`	`+`
`146`	`144`	`if ( P->mFeaturesCounted < mFetched )`
`147`	`145`	`{`
`148`	`146`	`QgsDebugMsg( QString( "feature count adjusted from %1 to %2" ).arg( P->mFeaturesCounted ).arg( mFetched ) );`
`@@ -179,8 +177,9 @@ bool QgsPostgresFeatureIterator::rewind()`
`179`	`177`	`return false;`
`180`	`178`
`181`	`179`	`// move cursor to first record`
`182`		`- P->mConnectionRO->PQexecNR( QString( "move 0 in %1" ).arg( mCursorName ) );`
	`180`	`+ P->mConnectionRO->PQexecNR( QString( "move absolute 0 in %1" ).arg( mCursorName ) );`
`183`	`181`	`mFeatureQueue.empty();`
	`182`	`+ mFetched = 0;`
`184`	`183`
`185`	`184`	`return true;`
`186`	`185`	`}`
`@@ -197,6 +196,7 @@ bool QgsPostgresFeatureIterator::close()`
`197`	`196`	`mFeatureQueue.dequeue();`
`198`	`197`	`}`
`199`	`198`
	`199`	`+ mClosed = true;`
`200`	`200`	`return true;`
`201`	`201`	`}`
`202`	`202`