Merge pull request #39942 from rouault/ogrprovider_featurecount

OGR provider: feature count improvements

python/core/auto_generated/qgsdataprovider.sip.in

@@ -73,6 +73,7 @@ Abstract base class for spatial data provider implementations.
     enum ReadFlag
     {
       FlagTrustDataSource,
+      SkipFeatureCount,
     };
     typedef QFlags<QgsDataProvider::ReadFlag> ReadFlags;
 

src/core/providers/ogr/qgsogrprovider.cpp

@@ -241,10 +241,14 @@ void QgsOgrProvider::repack()
 
   }
 
-  long oldcount = mFeaturesCounted;
-  recalculateFeatureCount();
-  if ( oldcount != mFeaturesCounted )
-    emit dataChanged();
+  if ( mFeaturesCounted != QgsVectorDataProvider::Uncounted &&
+       mFeaturesCounted != QgsVectorDataProvider::UnknownCount )
+  {
+    long oldcount = mFeaturesCounted;
+    recalculateFeatureCount();
+    if ( oldcount != mFeaturesCounted )
+      emit dataChanged();
+  }
 }
 
 
@@ -949,7 +953,8 @@ void QgsOgrProvider::addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer
 
 QStringList QgsOgrProvider::subLayers() const
 {
-  return _subLayers( true );
+  const bool withFeatureCount = ( mReadFlags & QgsDataProvider::SkipFeatureCount ) == 0;
+  return _subLayers( withFeatureCount );
 }
 
 QgsLayerMetadata QgsOgrProvider::layerMetadata() const
@@ -1303,7 +1308,7 @@ QString QgsOgrProvider::storageType() const
 }
 
 
-void QgsOgrProvider::setRelevantFields( bool fetchGeometry, const QgsAttributeList &fetchAttributes )
+void QgsOgrProvider::setRelevantFields( bool fetchGeometry, const QgsAttributeList &fetchAttributes ) const
 {
   QMutex *mutex = nullptr;
   OGRLayerH ogrLayer = mOgrLayer->getHandleAndMutex( mutex );
@@ -1600,6 +1605,15 @@ QgsWkbTypes::Type QgsOgrProvider::wkbType() const
  */
 long QgsOgrProvider::featureCount() const
 {
+  if ( ( mReadFlags & QgsDataProvider::SkipFeatureCount ) != 0 )
+  {
+    return QgsVectorDataProvider::UnknownCount;
+  }
+  if ( mRefreshFeatureCount )
+  {
+    mRefreshFeatureCount = false;
+    recalculateFeatureCount();
+  }
   return mFeaturesCounted;
 }
 
@@ -2317,12 +2331,7 @@ bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeature
 
   mOgrLayer->ResetReading();
 
-  // getting the total number of features in the layer
-  // TODO: This can be expensive, do we really need it!
-  if ( updateFeatureCount )
-  {
-    recalculateFeatureCount();
-  }
+  mRefreshFeatureCount = updateFeatureCount;
 
   // check the validity of the layer
   QgsDebugMsgLevel( QStringLiteral( "checking validity" ), 4 );
@@ -4643,7 +4652,7 @@ bool QgsOgrProvider::syncToDisc()
   return true;
 }
 
-void QgsOgrProvider::recalculateFeatureCount()
+void QgsOgrProvider::recalculateFeatureCount() const
 {
   if ( !mOgrLayer )
   {
@@ -4671,7 +4680,6 @@ void QgsOgrProvider::recalculateFeatureCount()
   else
   {
     mFeaturesCounted = 0;
-    mOgrLayer->ResetReading();
     setRelevantFields( true, QgsAttributeList() );
     mOgrLayer->ResetReading();
     gdal::ogr_feature_unique_ptr fet;
@@ -4688,7 +4696,7 @@ void QgsOgrProvider::recalculateFeatureCount()
       }
     }
     mOgrLayer->ResetReading();
-
+    setRelevantFields( true, attributeIndexes() );
   }
 
   if ( filter )
@@ -4906,7 +4914,7 @@ void QgsOgrProvider::open( OpenMode mode )
 
     // WARNING if this is the initial open - we don't already have a connection ref, and will be creating one later. So we *mustn't* grab an extra connection ref
     // while setting the subset string, or we'll be left with an extra reference which is never cleared.
-    mValid = _setSubsetString( origSubsetString, true, false, mode != OpenModeInitial );
+    mValid = _setSubsetString( origSubsetString, false, false, mode != OpenModeInitial );
 
     blockSignals( false );
     if ( mValid )
@@ -4968,15 +4976,12 @@ void QgsOgrProvider::open( OpenMode mode )
 
       if ( !mSubsetString.isEmpty() )
       {
-        int featuresCountedBackup = mFeaturesCounted;
-        mFeaturesCounted = -1;
         // Do not update capabilities here
         // but ensure subset is set (setSubsetString does nothing if the passed sql subset string is equal to
         // mSubsetString, which is the case when reloading the dataset)
         QString origSubsetString = mSubsetString;
         mSubsetString.clear();
         mValid = _setSubsetString( origSubsetString, false, false );
-        mFeaturesCounted = featuresCountedBackup;
       }
     }
   }
@@ -4988,6 +4993,8 @@ void QgsOgrProvider::open( OpenMode mode )
     setProperty( "_debug_open_mode", "read-write" );
   else
     setProperty( "_debug_open_mode", "read-only" );
+
+  mRefreshFeatureCount = true;
 }
 
 void QgsOgrProvider::close()

src/core/providers/ogr/qgsogrprovider.h

@@ -174,10 +174,10 @@ class QgsOgrProvider final: public QgsVectorDataProvider
     void loadFields();
 
     //! Find out the number of features of the whole layer
-    void recalculateFeatureCount();
+    void recalculateFeatureCount() const;
 
     //! Tell OGR, which fields to fetch in nextFeature/featureAtId (ie. which not to ignore)
-    void setRelevantFields( bool fetchGeometry, const QgsAttributeList &fetchAttributes );
+    void setRelevantFields( bool fetchGeometry, const QgsAttributeList &fetchAttributes ) const;
 
     //! Convert a QgsField to work with OGR
     static bool convertField( QgsField &field, const QTextCodec &encoding );
@@ -294,7 +294,11 @@ class QgsOgrProvider final: public QgsVectorDataProvider
     bool mValid = false;
 
     OGRwkbGeometryType mOGRGeomType = wkbUnknown;
-    long mFeaturesCounted = QgsVectorDataProvider::Uncounted;
+
+    //! Whether the next call to featureCount() should refresh the feature count
+    mutable bool mRefreshFeatureCount = true;
+
+    mutable long mFeaturesCounted = QgsVectorDataProvider::Uncounted;
 
     mutable QStringList mSubLayerList;
 

src/core/qgsdataprovider.h

@@ -122,6 +122,7 @@ class CORE_EXPORT QgsDataProvider : public QObject
     enum ReadFlag
     {
       FlagTrustDataSource = 1 << 0, //!< Trust datasource config (primary key unicity, geometry type and srid, etc). Improves provider load time by skipping expensive checks like primary key unicity, geometry type and srid and by using estimated metadata on data load. Since QGIS 3.16
+      SkipFeatureCount = 1 << 1, //!< Make featureCount() return -1 to indicate unknown, and subLayers() to return a unknown feature count as well. Since QGIS 3.18. Only implemented by OGR provider at time of writing.
     };
     Q_DECLARE_FLAGS( ReadFlags, ReadFlag )
 

tests/src/python/test_provider_shapefile.py

@@ -22,12 +22,14 @@
 from osgeo import gdal
 from qgis.core import (
     QgsApplication,
+    QgsDataProvider,
     QgsSettings,
     QgsFeature,
     QgsField,
     QgsGeometry,
     QgsVectorLayer,
     QgsFeatureRequest,
+    QgsProviderRegistry,
     QgsVectorDataProvider,
     QgsWkbTypes,
     QgsVectorLayerExporter,
@@ -1076,6 +1078,24 @@ def testEncoding(self):
         self.assertTrue(vl.isValid())
         self.assertEqual([f.attributes() for f in vl.dataProvider().getFeatures()], [['abcŐ'], ['abcŐabcŐabcŐ']])
 
+    def testSkipFeatureCountOnFeatureCount(self):
+        """Test QgsDataProvider.SkipFeatureCount on featureCount()"""
+
+        testPath = TEST_DATA_DIR + '/' + 'lines.shp'
+        provider = QgsProviderRegistry.instance().createProvider('ogr', testPath, QgsDataProvider.ProviderOptions(), QgsDataProvider.SkipFeatureCount)
+        self.assertTrue(provider.isValid())
+        self.assertEqual(provider.featureCount(), QgsVectorDataProvider.UnknownCount)
+
+    def testSkipFeatureCountOnSubLayers(self):
+        """Test QgsDataProvider.SkipFeatureCount on subLayers()"""
+
+        datasource = os.path.join(TEST_DATA_DIR, 'shapefile')
+        provider = QgsProviderRegistry.instance().createProvider('ogr', datasource, QgsDataProvider.ProviderOptions(), QgsDataProvider.SkipFeatureCount)
+        self.assertTrue(provider.isValid())
+        sublayers = provider.subLayers()
+        self.assertTrue(len(sublayers) > 1)
+        self.assertEqual(sublayers[0].split(QgsDataProvider.sublayerSeparator())[2], '-1')
+
 
 if __name__ == '__main__':
     unittest.main()