Skip to content
Permalink
Browse files

Fix caching of aggregate values with @parent

And add a test for referencedVariables
  • Loading branch information
m-kuhn committed Oct 28, 2016
1 parent 8e25b89 commit 7a05a7a8c4b9184f6cad7d155b6714076b1dccd9
Showing with 26 additions and 3 deletions.
  1. +15 −3 src/core/qgsexpression.cpp
  2. +11 −0 tests/src/core/testqgsexpression.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
@@ -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')" ) );

1 comment on commit 7a05a7a

@nirvn

This comment has been minimized.

Copy link
Contributor

@nirvn nirvn commented on 7a05a7a Oct 31, 2016

@m-kuhn , we're getting there! 😉 @parent is now properly updated as the code iterates through features. However, "FIELD" values (in the filter parameter expression) aren't updated when iterating through features, breaking stuff like aggregate('my_layer','count',$id,"my_layer_field" = attribute(@parent,"my_parent_field")).

Here's an updated test project (http://hub.qgis.org/attachments/10505/parent_v2.zip) which should help narrow things down. The test project labels two polygons using three aggregate calls. The third one is built on a "field" = attribute(@parent,"parent_field) which fails for the 2nd polygon. The failure is due to "field" not being updated with the 2nd polygon's values.

Screenshot:
lost

Please sign in to comment.
You can’t perform that action at this time.