Fix virtual fields which depend on other virtual fields may not be

calculated in some circumstances (fix #14939)

python/core/qgsfeaturerequest.sip

@@ -302,7 +302,7 @@ class QgsFeatureRequest
      * Return the subset of attributes which at least need to be fetched
      * @return A list of attributes to be fetched
      */
-    const QgsAttributeList& subsetOfAttributes() const;
+    QgsAttributeList subsetOfAttributes() const;
 
     //! Set a subset of attributes by names that will be fetched
     QgsFeatureRequest& setSubsetOfAttributes( const QStringList& attrNames, const QgsFields& fields );

src/core/qgsfeaturerequest.h

@@ -370,7 +370,7 @@ class CORE_EXPORT QgsFeatureRequest
      * Return the subset of attributes which at least need to be fetched
      * @return A list of attributes to be fetched
      */
-    const QgsAttributeList& subsetOfAttributes() const { return mAttrs; }
+    QgsAttributeList subsetOfAttributes() const { return mAttrs; }
 
     //! Set a subset of attributes by names that will be fetched
     QgsFeatureRequest& setSubsetOfAttributes( const QStringList& attrNames, const QgsFields& fields );

src/core/qgsvectorlayerfeatureiterator.cpp

@@ -527,6 +527,7 @@ void QgsVectorLayerFeatureIterator::prepareExpressions()
   mExpressionContext->appendScope( QgsExpressionContextUtils::projectScope() );
   mExpressionContext->setFields( mSource->mFields );
 
+  QList< int > virtualFieldsToFetch;
   for ( int i = 0; i < mSource->mFields.count(); i++ )
   {
     if ( mSource->mFields.fieldOrigin( i ) == QgsFields::OriginExpression )
@@ -535,7 +536,22 @@ void QgsVectorLayerFeatureIterator::prepareExpressions()
       if ( !( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
            || mRequest.subsetOfAttributes().contains( i ) )
       {
-        int oi = mSource->mFields.fieldOriginIndex( i );
+        virtualFieldsToFetch << i;
+      }
+    }
+  }
+
+  QList< int > virtualFieldsProcessed;
+  while ( !virtualFieldsToFetch.isEmpty() )
+  {
+    int fieldIdx = virtualFieldsToFetch.takeFirst();
+    if ( virtualFieldsProcessed.contains( fieldIdx ) )
+      continue;
+
+    virtualFieldsProcessed << fieldIdx;
+
+
+    int oi = mSource->mFields.fieldOriginIndex( fieldIdx );
     QgsExpression* exp = new QgsExpression( exps[oi].cachedExpression );
 
     QgsDistanceArea da;
@@ -547,17 +563,18 @@ void QgsVectorLayerFeatureIterator::prepareExpressions()
     exp->setAreaUnits( QgsProject::instance()->areaUnits() );
 
     exp->prepare( mExpressionContext.data() );
-        mExpressionFieldInfo.insert( i, exp );
+    mExpressionFieldInfo.insert( fieldIdx, exp );
 
-        if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
-        {
-          QgsAttributeList attrs;
     Q_FOREACH ( const QString& col, exp->referencedColumns() )
     {
-            attrs.append( mSource->mFields.fieldNameIndex( col ) );
+      int dependantFieldIdx = mSource->mFields.fieldNameIndex( col );
+      if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
+      {
+        mRequest.setSubsetOfAttributes( mRequest.subsetOfAttributes() << dependantFieldIdx );
       }
-
+      // also need to fetch this dependant field
-          mRequest.setSubsetOfAttributes( mRequest.subsetOfAttributes() + attrs );
+      if ( mSource->mFields.fieldOrigin( dependantFieldIdx ) == QgsFields::OriginExpression )
+        virtualFieldsToFetch << dependantFieldIdx;
     }
 
     if ( exp->needsGeometry() )
@@ -565,8 +582,7 @@ void QgsVectorLayerFeatureIterator::prepareExpressions()
       mRequest.setFlags( mRequest.flags() & ~QgsFeatureRequest::NoGeometry );
     }
   }
-    }
+
-  }
 }
 
 void QgsVectorLayerFeatureIterator::addJoinedAttributes( QgsFeature &f )

src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp

@@ -342,7 +342,7 @@ bool QgsDelimitedTextFeatureIterator::nextFeatureInternal( QgsFeature& feature )
 
     if ( ! mTestSubset && ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) )
     {
-      const QgsAttributeList& attrs = mRequest.subsetOfAttributes();
+      QgsAttributeList attrs = mRequest.subsetOfAttributes();
       for ( QgsAttributeList::const_iterator i = attrs.begin(); i != attrs.end(); ++i )
       {
         int fieldIdx = *i;

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -299,7 +299,7 @@ bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature )
   // fetch attributes
   if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
   {
-    const QgsAttributeList& attrs = mRequest.subsetOfAttributes();
+    QgsAttributeList attrs = mRequest.subsetOfAttributes();
     for ( QgsAttributeList::const_iterator it = attrs.begin(); it != attrs.end(); ++it )
     {
       getFeatureAttribute( fet, feature, *it );

src/providers/postgres/qgspostgresfeatureiterator.cpp

@@ -706,7 +706,7 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
   QgsFeatureId fid = 0;
 
   bool subsetOfAttributes = mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes;
-  const QgsAttributeList& fetchAttributes = mRequest.subsetOfAttributes();
+  QgsAttributeList fetchAttributes = mRequest.subsetOfAttributes();
 
   switch ( mSource->mPrimaryKeyType )
   {

src/providers/spatialite/qgsspatialitefeatureiterator.cpp

@@ -292,7 +292,7 @@ bool QgsSpatiaLiteFeatureIterator::prepareStatement( const QString& whereClause,
 
     if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
     {
-      const QgsAttributeList& fetchAttributes = mRequest.subsetOfAttributes();
+      QgsAttributeList fetchAttributes = mRequest.subsetOfAttributes();
       for ( QgsAttributeList::const_iterator it = fetchAttributes.constBegin(); it != fetchAttributes.constEnd(); ++it )
       {
         sql += ',' + fieldName( mSource->mFields.field( *it ) );

tests/src/python/test_qgsfeatureiterator.py

@@ -16,8 +16,10 @@ import qgis  # NOQA
 
 import os
 
-from qgis.core import QgsVectorLayer, QgsFeatureRequest, QgsFeature
+from qgis.core import QgsVectorLayer, QgsFeatureRequest, QgsFeature, QgsField, NULL
 from qgis.testing import start_app, unittest
+from qgis.PyQt.QtCore import QVariant
+
 from utilities import unitTestDataPath
 start_app()
 TEST_DATA_DIR = unitTestDataPath()
@@ -108,5 +110,52 @@ class TestQgsFeatureIterator(unittest.TestCase):
         feat['Staff'] = 2
         vl.addFeature(feat)
 
+    def test_ExpressionFieldNested(self):
+        myShpFile = os.path.join(TEST_DATA_DIR, 'points.shp')
+        layer = QgsVectorLayer(myShpFile, 'Points', 'ogr')
+        self.assertTrue(layer.isValid())
+
+        cnt = layer.pendingFields().count()
+        idx = layer.addExpressionField('"Staff"*2', QgsField('exp1', QVariant.LongLong))
+        idx = layer.addExpressionField('"exp1"-1', QgsField('exp2', QVariant.LongLong))
+
+        fet = next(layer.getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['exp2'], layer.fields())))
+        self.assertEqual(fet['Class'], NULL)
+        # nested virtual fields should make all these attributes be fetched
+        self.assertEqual(fet['Staff'], 2)
+        self.assertEqual(fet['exp2'], 3)
+        self.assertEqual(fet['exp1'], 4)
+
+    def test_ExpressionFieldNestedGeometry(self):
+        myShpFile = os.path.join(TEST_DATA_DIR, 'points.shp')
+        layer = QgsVectorLayer(myShpFile, 'Points', 'ogr')
+        self.assertTrue(layer.isValid())
+
+        cnt = layer.pendingFields().count()
+        idx = layer.addExpressionField('$x*2', QgsField('exp1', QVariant.LongLong))
+        idx = layer.addExpressionField('"exp1"/1.5', QgsField('exp2', QVariant.LongLong))
+
+        fet = next(layer.getFeatures(QgsFeatureRequest().setFlags(QgsFeatureRequest.NoGeometry).setSubsetOfAttributes(['exp2'], layer.fields())))
+        # nested virtual fields should have made geometry be fetched
+        self.assertEqual(fet['exp2'], -156)
+        self.assertEqual(fet['exp1'], -234)
+
+    def test_ExpressionFieldNestedCircular(self):
+        """ test circular virtual field definitions """
+
+        myShpFile = os.path.join(TEST_DATA_DIR, 'points.shp')
+        layer = QgsVectorLayer(myShpFile, 'Points', 'ogr')
+        self.assertTrue(layer.isValid())
+
+        cnt = layer.pendingFields().count()
+        idx = layer.addExpressionField('"exp3"*2', QgsField('exp1', QVariant.LongLong))
+        idx = layer.addExpressionField('"exp1"-1', QgsField('exp2', QVariant.LongLong))
+        idx = layer.addExpressionField('"exp2"*3', QgsField('exp3', QVariant.LongLong))
+
+        # really just testing that this doesn't hang/crash... there's no good result here!
+        fet = next(layer.getFeatures(QgsFeatureRequest().setSubsetOfAttributes(['exp2'], layer.fields())))
+        self.assertEqual(fet['Class'], NULL)
+
+
 if __name__ == '__main__':
     unittest.main()