Take advantage of pre-computed static expression nodes when determining

the referenced fields of an expression Avoids some cases where use of various expression functions which normally trigger all attributes to be requested, yet can be pre-computed during prepare stages, cause non-provider fields to be listed in the referenced columns and accordingly prevent expression compilation. Notably this can occur when using an expression like: aggregate( .... , filter:= "some_child_field"=attribute(@atlas_feature, 'some_atlas_field_name') ) where the whole attribute(@atlas_feature....) part is a constant static value and can be compiled down to a trivial, index-friendly "some_child_field"=### filter for the aggregate provider request. Ultimately giving a big performance boost to the atlas!
qgis · Feb 15, 2021 · 48ce042c84cdb27b9529c1d679f402cd15752271 · 48ce042
1 parent df30e64
commit 48ce042c84cdb27b9529c1d679f402cd15752271
diff --git a/python/core/auto_generated/expression/qgsexpression.sip.in b/python/core/auto_generated/expression/qgsexpression.sip.in
@@ -179,6 +179,17 @@ Gets list of columns referenced by the expression.
    all attributes from the layer are required for evaluation of the expression.
    :py:class:`QgsFeatureRequest`.setSubsetOfAttributes automatically handles this case.
 
+.. warning::
+
+   If the expression has been prepared via a call to :py:func:`QgsExpression.prepare()`,
+   or a call to :py:func:`QgsExpressionNode.prepare()` for a node has been made, then parts of
+   the expression may have been determined to evaluate to a static pre-calculatable value.
+   In this case the results will omit attribute indices which are used by these
+   pre-calculated nodes, regardless of their actual referenced columns.
+   If you are seeking to use these functions to introspect an expression you must
+   take care to do this with an unprepared expression.
+
+
 .. seealso:: :py:func:`referencedAttributeIndexes`
 %End
 
@@ -188,13 +199,25 @@ Returns a list of all variables which are used in this expression.
 If the list contains a NULL QString, there is a variable name used
 which is determined at runtime.
 
+.. note::
+
+   In contrast to the :py:func:`~QgsExpression.referencedColumns` function this method
+   is not affected by any previous calls to :py:func:`QgsExpression.prepare()`,
+   or :py:func:`QgsExpressionNode.prepare()`.
+
 .. versionadded:: 3.0
 %End
 
     QSet<QString> referencedFunctions() const;
 %Docstring
 Returns a list of the names of all functions which are used in this expression.
 
+.. note::
+
+   In contrast to the :py:func:`~QgsExpression.referencedColumns` function this method
+   is not affected by any previous calls to :py:func:`QgsExpression.prepare()`,
+   or :py:func:`QgsExpressionNode.prepare()`.
+
 .. versionadded:: 3.2
 %End
 
@@ -203,6 +226,16 @@ Returns a list of the names of all functions which are used in this expression.
 %Docstring
 Returns a list of field name indexes obtained from the provided fields.
 
+.. warning::
+
+   If the expression has been prepared via a call to :py:func:`QgsExpression.prepare()`,
+   or a call to :py:func:`QgsExpressionNode.prepare()` for a node has been made, then parts of
+   the expression may have been determined to evaluate to a static pre-calculatable value.
+   In this case the results will omit attribute indices which are used by these
+   pre-calculated nodes, regardless of their actual referenced columns.
+   If you are seeking to use these functions to introspect an expression you must
+   take care to do this with an unprepared expression.
+
 .. versionadded:: 3.0
 %End
 

diff --git a/python/core/auto_generated/expression/qgsexpressionnode.sip.in b/python/core/auto_generated/expression/qgsexpressionnode.sip.in
@@ -191,17 +191,37 @@ When reimplementing this, you need to return any column that is required to
 evaluate this node and in addition recursively collect all the columns required
 to evaluate child nodes.
 
+.. warning::
+
+   If the expression has been prepared via a call to :py:func:`QgsExpression.prepare()`,
+   or a call to :py:func:`QgsExpressionNode.prepare()` for a node has been made, then some nodes in
+   the expression may have been determined to evaluate to a static pre-calculatable value.
+   In this case the results will omit attribute indices which are used by these
+   pre-calculated nodes, regardless of their actual referenced columns.
+   If you are seeking to use these functions to introspect an expression you must
+   take care to do this with an unprepared expression node.
+
 :return: A list of columns required to evaluate this expression
 %End
 
     virtual QSet<QString> referencedVariables() const = 0;
 %Docstring
 Returns a set of all variables which are used in this expression.
+
+.. note::
+
+   In contrast to the :py:func:`~QgsExpressionNode.referencedColumns` function this method
+   is not affected by any previous calls to :py:func:`QgsExpressionNode.prepare()`.
 %End
 
     virtual QSet<QString> referencedFunctions() const = 0;
 %Docstring
 Returns a set of all functions which are used in this expression.
+
+.. note::
+
+   In contrast to the :py:func:`~QgsExpressionNode.referencedColumns` function this method
+   is not affected by any previous calls to :py:func:`QgsExpressionNode.prepare()`.
 %End
 
     virtual bool needsGeometry() const = 0;

diff --git a/src/core/expression/qgsexpression.h b/src/core/expression/qgsexpression.h
@@ -240,6 +240,14 @@ class CORE_EXPORT QgsExpression
      * all attributes from the layer are required for evaluation of the expression.
      * QgsFeatureRequest::setSubsetOfAttributes automatically handles this case.
      *
+     * \warning If the expression has been prepared via a call to QgsExpression::prepare(),
+     * or a call to QgsExpressionNode::prepare() for a node has been made, then parts of
+     * the expression may have been determined to evaluate to a static pre-calculatable value.
+     * In this case the results will omit attribute indices which are used by these
+     * pre-calculated nodes, regardless of their actual referenced columns.
+     * If you are seeking to use these functions to introspect an expression you must
+     * take care to do this with an unprepared expression.
+     *
      * \see referencedAttributeIndexes()
      */
     QSet<QString> referencedColumns() const;
@@ -249,13 +257,21 @@ class CORE_EXPORT QgsExpression
      * If the list contains a NULL QString, there is a variable name used
      * which is determined at runtime.
      *
+     * \note In contrast to the referencedColumns() function this method
+     * is not affected by any previous calls to QgsExpression::prepare(),
+     * or QgsExpressionNode::prepare().
+     *
      * \since QGIS 3.0
      */
     QSet<QString> referencedVariables() const;
 
     /**
      * Returns a list of the names of all functions which are used in this expression.
      *
+     * \note In contrast to the referencedColumns() function this method
+     * is not affected by any previous calls to QgsExpression::prepare(),
+     * or QgsExpressionNode::prepare().
+     *
      * \since QGIS 3.2
      */
     QSet<QString> referencedFunctions() const;
@@ -294,6 +310,14 @@ class CORE_EXPORT QgsExpression
     /**
      * Returns a list of field name indexes obtained from the provided fields.
      *
+     * \warning If the expression has been prepared via a call to QgsExpression::prepare(),
+     * or a call to QgsExpressionNode::prepare() for a node has been made, then parts of
+     * the expression may have been determined to evaluate to a static pre-calculatable value.
+     * In this case the results will omit attribute indices which are used by these
+     * pre-calculated nodes, regardless of their actual referenced columns.
+     * If you are seeking to use these functions to introspect an expression you must
+     * take care to do this with an unprepared expression.
+     *
      * \since QGIS 3.0
      */
     QSet<int> referencedAttributeIndexes( const QgsFields &fields ) const;

diff --git a/src/core/expression/qgsexpressionnode.h b/src/core/expression/qgsexpressionnode.h
@@ -219,17 +219,31 @@ class CORE_EXPORT QgsExpressionNode SIP_ABSTRACT
      * evaluate this node and in addition recursively collect all the columns required
      * to evaluate child nodes.
      *
+     * \warning If the expression has been prepared via a call to QgsExpression::prepare(),
+     * or a call to QgsExpressionNode::prepare() for a node has been made, then some nodes in
+     * the expression may have been determined to evaluate to a static pre-calculatable value.
+     * In this case the results will omit attribute indices which are used by these
+     * pre-calculated nodes, regardless of their actual referenced columns.
+     * If you are seeking to use these functions to introspect an expression you must
+     * take care to do this with an unprepared expression node.
+     *
      * \returns A list of columns required to evaluate this expression
      */
     virtual QSet<QString> referencedColumns() const = 0;
 
     /**
      * Returns a set of all variables which are used in this expression.
+     *
+     * \note In contrast to the referencedColumns() function this method
+     * is not affected by any previous calls to QgsExpressionNode::prepare().
      */
     virtual QSet<QString> referencedVariables() const = 0;
 
     /**
      * Returns a set of all functions which are used in this expression.
+     *
+     * \note In contrast to the referencedColumns() function this method
+     * is not affected by any previous calls to QgsExpressionNode::prepare().
      */
     virtual QSet<QString> referencedFunctions() const = 0;
 

diff --git a/src/core/expression/qgsexpressionnodeimpl.cpp b/src/core/expression/qgsexpressionnodeimpl.cpp
@@ -149,6 +149,9 @@ QString QgsExpressionNodeUnaryOperator::dump() const
 
 QSet<QString> QgsExpressionNodeUnaryOperator::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   return mOperand->referencedColumns();
 }
 
@@ -797,6 +800,9 @@ QString QgsExpressionNodeBinaryOperator::dump() const
 
 QSet<QString> QgsExpressionNodeBinaryOperator::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   return mOpLeft->referencedColumns() + mOpRight->referencedColumns();
 }
 
@@ -1031,6 +1037,9 @@ QString QgsExpressionNodeFunction::dump() const
 
 QSet<QString> QgsExpressionNodeFunction::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   QgsExpressionFunction *fd = QgsExpression::QgsExpression::Functions()[mFnIndex];
   QSet<QString> functionColumns = fd->referencedColumns( this );
 
@@ -1486,6 +1495,9 @@ QString QgsExpressionNodeCondition::dump() const
 
 QSet<QString> QgsExpressionNodeCondition::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   QSet<QString> lst;
   for ( WhenThen *cond : mConditions )
   {
@@ -1580,6 +1592,9 @@ bool QgsExpressionNodeCondition::isStatic( QgsExpression *parent, const QgsExpre
 
 QSet<QString> QgsExpressionNodeInOperator::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   QSet<QString> lst( mNode->referencedColumns() );
   const QList< QgsExpressionNode * > nodeList = mList->list();
   for ( const QgsExpressionNode *n : nodeList )
@@ -1694,6 +1709,9 @@ QString QgsExpressionNodeIndexOperator::dump() const
 
 QSet<QString> QgsExpressionNodeIndexOperator::referencedColumns() const
 {
+  if ( hasCachedStaticValue() )
+    return QSet< QString >();
+
   return mContainer->referencedColumns() + mIndex->referencedColumns();
 }
 

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -4135,6 +4135,66 @@ class TestQgsExpression: public QObject
       }
     }
 
+    void testPrecomputedNodesWithIntrospectionFunctions()
+    {
+      QgsFields fields;
+      fields.append( QgsField( QStringLiteral( "first_field" ), QVariant::Int ) );
+      fields.append( QgsField( QStringLiteral( "second_field" ), QVariant::Int ) );
+
+      QgsExpression exp( QStringLiteral( "attribute(@static_feature, concat('second','_',@field_name_part_var)) + x(geometry( @static_feature ))" ) );
+      // initially this expression requires all attributes -- we can't determine the referenced columns in advance
+      QCOMPARE( exp.referencedColumns(), QSet<QString>() << QgsFeatureRequest::ALL_ATTRIBUTES );
+      QCOMPARE( exp.referencedAttributeIndexes( fields ), QSet< int >() << 0 << 1 );
+      QCOMPARE( exp.referencedFunctions(), QSet<QString>() << QStringLiteral( "attribute" ) << QStringLiteral( "concat" ) << QStringLiteral( "geometry" ) << QStringLiteral( "x" ) << QStringLiteral( "var" ) );
+      QCOMPARE( exp.referencedVariables(), QSet<QString>() << QStringLiteral( "field_name_part_var" ) << QStringLiteral( "static_feature" ) );
+
+      // prepare the expression using static variables
+      QgsExpressionContext context;
+      std::unique_ptr< QgsExpressionContextScope > scope = qgis::make_unique< QgsExpressionContextScope >();
+      scope->setVariable( QStringLiteral( "field_name_part_var" ), QStringLiteral( "field" ), true );
+
+      // this feature gets added as a static variable, to emulate eg the @atlas_feature variable
+      QgsFeature feature( fields );
+      feature.setAttributes( QgsAttributes() << 5 << 10 );
+      feature.setGeometry( QgsGeometry::fromPointXY( QgsPointXY( 27, 42 ) ) );
+      scope->setVariable( QStringLiteral( "static_feature" ), feature, true );
+
+      context.appendScope( scope.release() );
+
+      QVERIFY( exp.prepare( & context ) );
+      // because all parts of the expression are static, the root node should have a cached static value!
+      QVERIFY( exp.rootNode()->hasCachedStaticValue() );
+      QCOMPARE( exp.rootNode()->cachedStaticValue().toInt(), 37 );
+
+      // referenced columns should be empty -- we don't need ANY columns to evaluate this expression
+      QVERIFY( exp.referencedColumns().empty() );
+      QVERIFY( exp.referencedAttributeIndexes( fields ).empty() );
+      // in contrast, referencedFunctions() and referencedVariables() should NOT be affected by pre-compiled nodes
+      // as these methods are used for introspection purposes only...
+      QCOMPARE( exp.referencedFunctions(), QSet<QString>() << QStringLiteral( "attribute" ) << QStringLiteral( "concat" ) << QStringLiteral( "geometry" ) << QStringLiteral( "x" ) << QStringLiteral( "var" ) );
+      QCOMPARE( exp.referencedVariables(), QSet<QString>() << QStringLiteral( "field_name_part_var" ) << QStringLiteral( "static_feature" ) );
+
+      // secondary test - this one uses a mix of pre-computable nodes and non-pre-computable nodes
+      QgsExpression exp2( QStringLiteral( "(attribute(@static_feature, concat('second','_',@field_name_part_var)) + x(geometry( @static_feature ))) > \"another_field\"" ) );
+      QCOMPARE( exp2.referencedColumns(), QSet<QString>() << QgsFeatureRequest::ALL_ATTRIBUTES << QStringLiteral( "another_field" ) );
+      QCOMPARE( exp2.referencedFunctions(), QSet<QString>() << QStringLiteral( "attribute" ) << QStringLiteral( "concat" ) << QStringLiteral( "geometry" ) << QStringLiteral( "x" ) << QStringLiteral( "var" ) );
+      QCOMPARE( exp2.referencedVariables(), QSet<QString>() << QStringLiteral( "field_name_part_var" ) << QStringLiteral( "static_feature" ) );
+
+      QgsFields fields2;
+      fields2.append( QgsField( QStringLiteral( "another_field" ), QVariant::Int ) );
+      context.setFields( fields2 );
+
+      QVERIFY( exp2.prepare( & context ) );
+      // because NOT all parts of the expression are static, the root node should NOT have a cached static value!
+      QVERIFY( !exp2.rootNode()->hasCachedStaticValue() );
+
+      // but the only referenced column should be "another_field", because the first half of the expression with the "attribute" function is static and has been precomputed
+      QCOMPARE( exp2.referencedColumns(), QSet<QString>() << QStringLiteral( "another_field" ) );
+      QCOMPARE( exp2.referencedAttributeIndexes( fields2 ), QSet< int >() << 0 );
+      QCOMPARE( exp2.referencedFunctions(), QSet<QString>() << QStringLiteral( "attribute" ) << QStringLiteral( "concat" ) << QStringLiteral( "geometry" ) << QStringLiteral( "x" ) << QStringLiteral( "var" ) );
+      QCOMPARE( exp2.referencedVariables(), QSet<QString>() << QStringLiteral( "field_name_part_var" ) << QStringLiteral( "static_feature" ) );
+    }
+
 };
 
 QGSTEST_MAIN( TestQgsExpression )