From 3f4d6de54b476176724877c5a490665fa3dcb9df Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 13 Jul 2017 10:58:04 +1000 Subject: [PATCH 1/2] Fix aggregate expression calculation when used with virtual fields The layer expression context (which is required for aggregate calculation to work) was not being added to the context used by vector layer feature iterators. Fix #15930 --- python/core/qgsvectorlayerfeatureiterator.sip | 1 + src/core/qgsvectorlayerfeatureiterator.cpp | 5 ++++- src/core/qgsvectorlayerfeatureiterator.h | 2 ++ tests/src/python/test_qgsvectorlayer.py | 21 +++++++++++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/python/core/qgsvectorlayerfeatureiterator.sip b/python/core/qgsvectorlayerfeatureiterator.sip index 40f6aaceb2f..30340517ab3 100644 --- a/python/core/qgsvectorlayerfeatureiterator.sip +++ b/python/core/qgsvectorlayerfeatureiterator.sip @@ -61,6 +61,7 @@ class QgsVectorLayerFeatureSource : QgsAbstractFeatureSource + }; diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp index ad6acfca0d5..9168704b1dd 100644 --- a/src/core/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/qgsvectorlayerfeatureiterator.cpp @@ -79,6 +79,9 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( const QgsVectorLayer * } #endif } + + std::unique_ptr< QgsExpressionContextScope > layerScope( QgsExpressionContextUtils::layerScope( layer ) ); + mLayerScope = *layerScope; } QgsVectorLayerFeatureSource::~QgsVectorLayerFeatureSource() @@ -644,7 +647,7 @@ void QgsVectorLayerFeatureIterator::prepareFields() mExpressionContext.reset( new QgsExpressionContext() ); mExpressionContext->appendScope( QgsExpressionContextUtils::globalScope() ); mExpressionContext->appendScope( QgsExpressionContextUtils::projectScope( QgsProject::instance() ) ); - mExpressionContext->setFields( mSource->mFields ); + mExpressionContext->appendScope( new QgsExpressionContextScope( mSource->mLayerScope ) ); mFieldsToPrepare = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList(); diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h index 0c6cd50d99e..8c3392ca111 100644 --- a/src/core/qgsvectorlayerfeatureiterator.h +++ b/src/core/qgsvectorlayerfeatureiterator.h @@ -84,6 +84,8 @@ class CORE_EXPORT QgsVectorLayerFeatureSource : public QgsAbstractFeatureSource QgsFields mFields; + QgsExpressionContextScope mLayerScope; + bool mHasEditBuffer; // A deep-copy is only performed, if the original maps change diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index bbce317f9c9..5fdcabb1300 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -1829,6 +1829,27 @@ class TestQgsVectorLayer(unittest.TestCase, FeatureSourceTestCase): self.assertTrue(ok) self.assertEqual(val, 'this is a test') + def testAggregateInVirtualField(self): + """ + Test aggregates in a virtual field + """ + layer = QgsVectorLayer("Point?field=fldint:integer", "layer", "memory") + pr = layer.dataProvider() + + int_values = [4, 2, 3, 2, 5, None, 8] + features = [] + for i in int_values: + f = QgsFeature() + f.setFields(layer.fields()) + f.setAttributes([i]) + features.append(f) + assert pr.addFeatures(features) + + field = QgsField('virtual', QVariant.Double) + layer.addExpressionField('sum(fldint*2)', field) + vals = [f['virtual'] for f in layer.getFeatures()] + self.assertEqual(vals, [48, 48, 48, 48, 48, 48, 48]) + def onLayerOpacityChanged(self, tr): self.opacityTest = tr From 8711473b7f83d436eda5e7f4627a6090f2a227bc Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 13 Jul 2017 11:01:55 +1000 Subject: [PATCH 2/2] Add a very basic guard against virtual fields which reference themself --- src/core/qgsvectorlayerfeatureiterator.cpp | 9 +++++++++ tests/src/python/test_qgsvectorlayer.py | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp index 9168704b1dd..08f33e105f2 100644 --- a/src/core/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/qgsvectorlayerfeatureiterator.cpp @@ -617,6 +617,15 @@ void QgsVectorLayerFeatureIterator::prepareExpression( int fieldIdx ) exp->setAreaUnits( QgsProject::instance()->areaUnits() ); exp->prepare( mExpressionContext.get() ); + Q_FOREACH ( const QString &col, exp->referencedColumns() ) + { + if ( mSource->fields().lookupField( col ) == fieldIdx ) + { + // circular reference - expression depends on column itself + delete exp; + return; + } + } mExpressionFieldInfo.insert( fieldIdx, exp ); Q_FOREACH ( const QString &col, exp->referencedColumns() ) diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 5fdcabb1300..30fe77f7e80 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -1609,6 +1609,11 @@ class TestQgsVectorLayer(unittest.TestCase, FeatureSourceTestCase): self.assertEqual(layer.pendingFields().count(), cnt) + # expression field which references itself + idx = layer.addExpressionField('sum(test2)', QgsField('test2', QVariant.LongLong)) + fet = next(layer.getFeatures()) + self.assertEqual(fet['test2'], NULL) + def test_ExpressionFieldEllipsoidLengthCalculation(self): #create a temporary layer temp_layer = QgsVectorLayer("LineString?crs=epsg:3111&field=pk:int", "vl", "memory")