Fix caching of aggregate values with @parent

m-kuhn · m-kuhn · commit 7a05a7a8c4b9 · 2016-10-28T12:31:40.000+02:00
And add a test for referencedVariables
diff --git a/src/core/qgsexpression.cpp b/src/core/qgsexpression.cpp
@@ -682,8 +682,8 @@ static QVariant fcnAggregate( const QVariantList& values, const QgsExpressionCon
   {
     QString cacheKey = QStringLiteral( "aggfcn:%1:%2:%3:%4" ).arg( vl->id(), QString::number( aggregate ), subExpression, parameters.filter );
 
-    QgsExpression subExp( subExpression );
-    if ( subExp.referencedVariables().contains( "parent" ) || subExp.referencedVariables().contains( QString() ) )
+    QgsExpression filterExp( parameters.filter );
+    if ( filterExp.referencedVariables().contains( "parent" ) || filterExp.referencedVariables().contains( QString() ) )
     {
       cacheKey += ':' + qHash( context->feature() );
     }
@@ -5106,7 +5106,19 @@ QSet<QString> QgsExpression::NodeFunction::referencedVariables() const
     return QSet<QString>() << QString();
   }
   else
-    return QSet<QString>();
+  {
+    QSet<QString> functionVariables = QSet<QString>();
+
+    if ( !mArgs )
+      return functionVariables;
+
+    Q_FOREACH ( Node* n, mArgs->list() )
+    {
+      functionVariables.unite( n->referencedVariables() );
+    }
+
+    return functionVariables;
+  }
 }
 
 bool QgsExpression::NodeFunction::needsGeometry() const
diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -1571,6 +1571,17 @@ class TestQgsExpression: public QObject
       QCOMPARE( refColsSet, expectedCols );
     }
 
+    void referenced_variables()
+    {
+      QSet<QString> expectedVars;
+      expectedVars << QStringLiteral( "foo" ) << QStringLiteral( "bar" ) << QStringLiteral( "ppp" ) << QStringLiteral( "qqq" ) << QStringLiteral( "rrr" );
+      QgsExpression exp( QStringLiteral( "CASE WHEN intersects(@bar, $geometry) THEN @ppp ELSE @qqq * @rrr END + @foo IN (1, 2, 3)" ) );
+      QCOMPARE( exp.hasParserError(), false );
+      QSet<QString> refVar = exp.referencedVariables();
+
+      QCOMPARE( refVar, expectedVars );
+    }
+
     void referenced_columns_all_attributes()
     {
       QgsExpression exp( QStringLiteral( "attribute($currentfeature,'test')" ) );

Original file line number	Diff line number	Diff line change
`@@ -682,8 +682,8 @@ static QVariant fcnAggregate( const QVariantList& values, const QgsExpressionCon`
`682`	`682`	`{`
`683`	`683`	`QString cacheKey = QStringLiteral( "aggfcn:%1:%2:%3:%4" ).arg( vl->id(), QString::number( aggregate ), subExpression, parameters.filter );`
`684`	`684`
`685`		`- QgsExpression subExp( subExpression );`
`686`		`- if ( subExp.referencedVariables().contains( "parent" ) \|\| subExp.referencedVariables().contains( QString() ) )`
	`685`	`+ QgsExpression filterExp( parameters.filter );`
	`686`	`+ if ( filterExp.referencedVariables().contains( "parent" ) \|\| filterExp.referencedVariables().contains( QString() ) )`
`687`	`687`	`{`
`688`	`688`	`cacheKey += ':' + qHash( context->feature() );`
`689`	`689`	`}`
`@@ -5106,7 +5106,19 @@ QSet<QString> QgsExpression::NodeFunction::referencedVariables() const`
`5106`	`5106`	`return QSet<QString>() << QString();`
`5107`	`5107`	`}`
`5108`	`5108`	`else`
`5109`		`- return QSet<QString>();`
	`5109`	`+ {`
	`5110`	`+ QSet<QString> functionVariables = QSet<QString>();`
	`5111`	`+`
	`5112`	`+ if ( !mArgs )`
	`5113`	`+ return functionVariables;`
	`5114`	`+`
	`5115`	`+ Q_FOREACH ( Node* n, mArgs->list() )`
	`5116`	`+ {`
	`5117`	`+ functionVariables.unite( n->referencedVariables() );`
	`5118`	`+ }`
	`5119`	`+`
	`5120`	`+ return functionVariables;`
	`5121`	`+ }`
`5110`	`5122`	`}`
`5111`	`5123`
`5112`	`5124`	`bool QgsExpression::NodeFunction::needsGeometry() const`