Fix calculation of relation aggregate in virtual field with same name…

… as related field
qgis · Jun 20, 2018 · 77d2284 · 77d2284
1 parent 4132dbc
commit 77d2284
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 7 deletions.
diff --git a/python/core/auto_generated/expression/qgsexpressionfunction.sip.in b/python/core/auto_generated/expression/qgsexpressionfunction.sip.in
@@ -38,13 +38,15 @@ Represents a single parameter passed to a function.
 
         Parameter( const QString &name,
                    bool optional = false,
-                   const QVariant &defaultValue = QVariant() );
+                   const QVariant &defaultValue = QVariant(),
+                   bool isSubExpression = false );
 %Docstring
 Constructor for Parameter.
 
 :param name: parameter name, used when named parameter are specified in an expression
 :param optional: set to true if parameter should be optional
 :param defaultValue: default value to use for optional parameters
+:param isSubExpression: set to true if this parameter is a sub-expression
 %End
 
         QString name() const;
@@ -60,6 +62,14 @@ Returns true if the parameter is optional.
         QVariant defaultValue() const;
 %Docstring
 Returns the default value for the parameter.
+%End
+
+        bool isSubExpression() const;
+%Docstring
+Returns true if parameter argument is a separate sub-expression, and
+should not be checked while determining referenced columns for the expression.
+
+.. versionadded:: 3.2
 %End
 
         bool operator==( const QgsExpressionFunction::Parameter &other ) const;

diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -4113,8 +4113,8 @@ const QList<QgsExpressionFunction *> &QgsExpression::Functions()
                                             QgsExpressionFunction::ParameterList()
                                             << QgsExpressionFunction::Parameter( QStringLiteral( "layer" ) )
                                             << QgsExpressionFunction::Parameter( QStringLiteral( "aggregate" ) )
-                                            << QgsExpressionFunction::Parameter( QStringLiteral( "expression" ) )
-                                            << QgsExpressionFunction::Parameter( QStringLiteral( "filter" ), true )
+                                            << QgsExpressionFunction::Parameter( QStringLiteral( "expression" ), false, QVariant(), true )
+                                            << QgsExpressionFunction::Parameter( QStringLiteral( "filter" ), true, QVariant(), true )
                                             << QgsExpressionFunction::Parameter( QStringLiteral( "concatenator" ), true ),
                                             fcnAggregate,
                                             QStringLiteral( "Aggregates" ),
@@ -4177,7 +4177,7 @@ const QList<QgsExpressionFunction *> &QgsExpression::Functions()
     true
                                           )
 
-        << new QgsStaticExpressionFunction( QStringLiteral( "relation_aggregate" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "relation" ) ) << QgsExpressionFunction::Parameter( QStringLiteral( "aggregate" ) ) << QgsExpressionFunction::Parameter( QStringLiteral( "expression" ) ) << QgsExpressionFunction::Parameter( QStringLiteral( "concatenator" ), true ),
+        << new QgsStaticExpressionFunction( QStringLiteral( "relation_aggregate" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "relation" ) ) << QgsExpressionFunction::Parameter( QStringLiteral( "aggregate" ) ) << QgsExpressionFunction::Parameter( QStringLiteral( "expression" ), false, QVariant(), true ) << QgsExpressionFunction::Parameter( QStringLiteral( "concatenator" ), true ),
                                             fcnAggregateRelation, QStringLiteral( "Aggregates" ), QString(), false, QSet<QString>() << QgsFeatureRequest::ALL_ATTRIBUTES, true )
 
         << new QgsStaticExpressionFunction( QStringLiteral( "count" ), aggParams, fcnAggregateCount, QStringLiteral( "Aggregates" ), QString(), false, QSet<QString>(), true )

diff --git a/src/core/expression/qgsexpressionfunction.h b/src/core/expression/qgsexpressionfunction.h
@@ -58,13 +58,16 @@ class CORE_EXPORT QgsExpressionFunction
          * \param name parameter name, used when named parameter are specified in an expression
          * \param optional set to true if parameter should be optional
          * \param defaultValue default value to use for optional parameters
+         * \param isSubExpression set to true if this parameter is a sub-expression
          */
         Parameter( const QString &name,
                    bool optional = false,
-                   const QVariant &defaultValue = QVariant() )
+                   const QVariant &defaultValue = QVariant(),
+                   bool isSubExpression = false )
           : mName( name )
           , mOptional( optional )
           , mDefaultValue( defaultValue )
+          , mIsSubExpression( isSubExpression )
         {}
 
         //! Returns the name of the parameter.
@@ -76,15 +79,23 @@ class CORE_EXPORT QgsExpressionFunction
         //! Returns the default value for the parameter.
         QVariant defaultValue() const { return mDefaultValue; }
 
+        /**
+         * Returns true if parameter argument is a separate sub-expression, and
+         * should not be checked while determining referenced columns for the expression.
+         * \since QGIS 3.2
+         */
+        bool isSubExpression() const { return mIsSubExpression; }
+
         bool operator==( const QgsExpressionFunction::Parameter &other ) const
         {
           return ( QString::compare( mName, other.mName, Qt::CaseInsensitive ) == 0 );
         }
 
       private:
         QString mName;
-        bool mOptional;
+        bool mOptional = false;
         QVariant mDefaultValue;
+        bool mIsSubExpression = false;
     };
 
     //! List of parameters, used for function definition

diff --git a/src/core/expression/qgsexpressionnodeimpl.cpp b/src/core/expression/qgsexpressionnodeimpl.cpp
@@ -965,10 +965,13 @@ QSet<QString> QgsExpressionNodeFunction::referencedColumns() const
     return functionColumns;
   }
 
+  int paramIndex = 0;
   const QList< QgsExpressionNode * > nodeList = mArgs->list();
   for ( QgsExpressionNode *n : nodeList )
   {
-    functionColumns.unite( n->referencedColumns() );
+    if ( fd->parameters().count() <= paramIndex || !fd->parameters().at( paramIndex ).isSubExpression() )
+      functionColumns.unite( n->referencedColumns() );
+    paramIndex++;
   }
 
   return functionColumns;

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -1843,6 +1843,16 @@ class TestQgsExpression: public QObject
         refColsSet.insert( col.toLower() );
 
       QCOMPARE( refColsSet, expectedCols );
+
+      expectedCols.clear();
+      expectedCols << QgsFeatureRequest::ALL_ATTRIBUTES
+                   << QStringLiteral( "parent_col1" )
+                   << QStringLiteral( "parent_col2" );
+      // sub expression fields, "child_field", "child_field2" should not be included in referenced columns
+      exp = QgsExpression( QStringLiteral( "relation_aggregate(relation:=\"parent_col1\" || 'my_rel',aggregate:='sum' || \"parent_col2\", expression:=\"child_field\" * \"child_field2\")" ) );
+      QCOMPARE( exp.hasParserError(), false );
+      refCols = exp.referencedColumns();
+      QCOMPARE( refCols, expectedCols );
     }
 
     void referenced_variables()