Skip to content
Permalink
Browse files
[expressions] Don't cache aggregate results if there's an evaluation …
…error.

Otherwise subsequent evaluations of the expression will happily
grab the cached null variant, unaware that it's actually a
result of a broken expression and not a valid null aggregate
calculation.

(cherry picked from commit 97f0242)
  • Loading branch information
nyalldawson committed Aug 17, 2021
1 parent 1cb8df8 commit 49c150cc2896cebcf3715dbe0b3d348facc9d508
Showing with 9 additions and 1 deletion.
  1. +7 −1 src/core/expression/qgsexpressionfunction.cpp
  2. +2 −0 tests/src/core/testqgsexpression.cpp
@@ -681,7 +681,13 @@ static QVariant fcnAggregate( const QVariantList &values, const QgsExpressionCon
subContext.appendScope( subScope );
result = vl->aggregate( aggregate, subExpression, parameters, &subContext, &ok );

context->setCachedValue( cacheKey, result );
if ( ok )
{
// important -- we should only store cached values when the expression is successfully calculated. Otherwise subsequent
// use of the expression context will happily grab the invalid QVariant cached value without realising that there was actually an error
// associated with it's calculation!
context->setCachedValue( cacheKey, result );
}
}
else
{
@@ -2154,6 +2154,8 @@ class TestQgsExpression: public QObject

// check again - make sure value was correctly cached
res = exp.evaluate( &context );
// if first evaluation has an eval error, so should any subsequent evaluations!
QCOMPARE( exp.hasEvalError(), evalError );
QCOMPARE( res, result );
}

0 comments on commit 49c150c

Please sign in to comment.