Copy error message from aggregate calculator to expression when

aggregate function fails, so that we give more helpful error messages for debugging aggregate based expressions
qgis · Aug 9, 2021 · bf6af35 · bf6af35
1 parent f3c3fed
commit bf6af35
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 9 deletions.
diff --git a/python/core/auto_generated/qgsaggregatecalculator.sip.in b/python/core/auto_generated/qgsaggregatecalculator.sip.in
@@ -75,6 +75,13 @@ to a data provider for remote calculation.
 Constructor for QgsAggregateCalculator.
 
 :param layer: vector layer to calculate aggregate from
+%End
+
+    QString lastError() const;
+%Docstring
+Returns the last error encountered during the aggregate calculation.
+
+.. versionadded:: 3.22
 %End
 
     const QgsVectorLayer *layer() const;
@@ -140,7 +147,7 @@ Calculates the value of an aggregate.
 :param fieldOrExpression: source field or expression to use as basis for aggregated values.
                           If an expression is used, then the context parameter must be set.
 :param context: expression context for evaluating expressions
-:param ok: if specified, will be set to ``True`` if aggregate calculation was successful
+:param ok: if specified, will be set to ``True`` if aggregate calculation was successful. If \ok is ``False`` then :py:func:`~QgsAggregateCalculator.lastError` can be used to retrieve a descriptive error message.
 :param feedback: optional feedback argument for early cancellation (since QGIS 3.22). If set, this will take precedence over any feedback object
                  set on the expression ``context``.
 

diff --git a/python/core/auto_generated/vector/qgsvectorlayer.sip.in b/python/core/auto_generated/vector/qgsvectorlayer.sip.in
@@ -2461,7 +2461,8 @@ and maximum values are required.
                         QgsExpressionContext *context = 0,
                         bool *ok = 0,
                         QgsFeatureIds *fids = 0,
-                        QgsFeedback *feedback = 0 ) const;
+                        QgsFeedback *feedback = 0) const;
+
 %Docstring
 Calculates an aggregated value from the layer's features.
 Currently any filtering expression provided will override filters in the FeatureRequest.

diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -638,6 +638,7 @@ static QVariant fcnAggregate( const QVariantList &values, const QgsExpressionCon
     }
   }
 
+  QString aggregateError;
   QVariant result;
   if ( context )
   {
@@ -686,7 +687,7 @@ static QVariant fcnAggregate( const QVariantList &values, const QgsExpressionCon
     QgsExpressionContextScope *subScope = new QgsExpressionContextScope();
     subScope->setVariable( QStringLiteral( "parent" ), context->feature() );
     subContext.appendScope( subScope );
-    result = vl->aggregate( aggregate, subExpression, parameters, &subContext, &ok, nullptr, context->feedback() );
+    result = vl->aggregate( aggregate, subExpression, parameters, &subContext, &ok, nullptr, context->feedback(), &aggregateError );
 
     if ( ok )
     {
@@ -698,11 +699,14 @@ static QVariant fcnAggregate( const QVariantList &values, const QgsExpressionCon
   }
   else
   {
-    result = vl->aggregate( aggregate, subExpression, parameters, nullptr, &ok );
+    result = vl->aggregate( aggregate, subExpression, parameters, nullptr, &ok, nullptr, nullptr, &aggregateError );
   }
   if ( !ok )
   {
-    parent->setEvalErrorString( QObject::tr( "Could not calculate aggregate for: %1" ).arg( subExpression ) );
+    if ( !aggregateError.isEmpty() )
+      parent->setEvalErrorString( QObject::tr( "Could not calculate aggregate for: %1 (%2)" ).arg( subExpression, aggregateError ) );
+    else
+      parent->setEvalErrorString( QObject::tr( "Could not calculate aggregate for: %1" ).arg( subExpression ) );
     return QVariant();
   }
 

diff --git a/src/core/qgsaggregatecalculator.cpp b/src/core/qgsaggregatecalculator.cpp
@@ -51,6 +51,7 @@ void QgsAggregateCalculator::setFidsFilter( const QgsFeatureIds &fids )
 QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate aggregate,
     const QString &fieldOrExpression, QgsExpressionContext *context, bool *ok, QgsFeedback *feedback ) const
 {
+  mLastError.clear();
   if ( ok )
     *ok = false;
 
@@ -74,7 +75,10 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
     expression.reset( new QgsExpression( fieldOrExpression ) );
 
     if ( expression->hasParserError() || !expression->prepare( context ) )
+    {
+      mLastError = !expression->parserErrorString().isEmpty() ? expression->parserErrorString() : expression->evalErrorString();
       return QVariant();
+    }
   }
 
   QSet<QString> lst;

diff --git a/src/core/qgsaggregatecalculator.h b/src/core/qgsaggregatecalculator.h
@@ -122,6 +122,13 @@ class CORE_EXPORT QgsAggregateCalculator
      */
     QgsAggregateCalculator( const QgsVectorLayer *layer );
 
+    /**
+     * Returns the last error encountered during the aggregate calculation.
+     *
+     * \since QGIS 3.22
+     */
+    QString lastError() const { return mLastError; }
+
     /**
      * Returns the associated vector layer.
      */
@@ -173,7 +180,7 @@ class CORE_EXPORT QgsAggregateCalculator
      * \param fieldOrExpression source field or expression to use as basis for aggregated values.
      * If an expression is used, then the context parameter must be set.
      * \param context expression context for evaluating expressions
-     * \param ok if specified, will be set to TRUE if aggregate calculation was successful
+     * \param ok if specified, will be set to TRUE if aggregate calculation was successful. If \ok is FALSE then lastError() can be used to retrieve a descriptive error message.
      * \param feedback optional feedback argument for early cancellation (since QGIS 3.22). If set, this will take precedence over any feedback object
      * set on the expression \a context.
      * \returns calculated aggregate value
@@ -216,6 +223,8 @@ class CORE_EXPORT QgsAggregateCalculator
     //trigger variable
     bool mFidsSet = false;
 
+    mutable QString mLastError;
+
     static QgsStatisticalSummary::Statistic numericStatFromAggregate( Aggregate aggregate, bool *ok = nullptr );
     static QgsStringStatisticalSummary::Statistic stringStatFromAggregate( Aggregate aggregate, bool *ok = nullptr );
     static QgsDateTimeStatisticalSummary::Statistic dateTimeStatFromAggregate( Aggregate aggregate, bool *ok = nullptr );

diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp
@@ -4440,13 +4440,17 @@ void QgsVectorLayer::minimumOrMaximumValue( int index, QVariant *minimum, QVaria
 
 QVariant QgsVectorLayer::aggregate( QgsAggregateCalculator::Aggregate aggregate, const QString &fieldOrExpression,
                                     const QgsAggregateCalculator::AggregateParameters &parameters, QgsExpressionContext *context,
-                                    bool *ok, QgsFeatureIds *fids, QgsFeedback *feedback ) const
+                                    bool *ok, QgsFeatureIds *fids, QgsFeedback *feedback, QString *error ) const
 {
   if ( ok )
     *ok = false;
+  if ( error )
+    error->clear();
 
   if ( !mDataProvider )
   {
+    if ( error )
+      *error = tr( "Layer is invalid" );
     return QVariant();
   }
 
@@ -4476,7 +4480,14 @@ QVariant QgsVectorLayer::aggregate( QgsAggregateCalculator::Aggregate aggregate,
   if ( fids )
     c.setFidsFilter( *fids );
   c.setParameters( parameters );
-  return c.calculate( aggregate, fieldOrExpression, context, ok, feedback );
+  bool aggregateOk = false;
+  const QVariant result = c.calculate( aggregate, fieldOrExpression, context, &aggregateOk, feedback );
+  if ( ok )
+    *ok = aggregateOk;
+  if ( !aggregateOk && error )
+    *error = c.lastError();
+
+  return result;
 }
 
 void QgsVectorLayer::setFeatureBlendMode( QPainter::CompositionMode featureBlendMode )

diff --git a/src/core/vector/qgsvectorlayer.h b/src/core/vector/qgsvectorlayer.h
@@ -2256,6 +2256,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
      * \param ok if specified, will be set to TRUE if aggregate calculation was successful
      * \param fids list of fids to filter, otherwise will use all fids
      * \param feedback optional feedback argument for early cancellation (since QGIS 3.22)
+     * \param error optional storage for error messages (not available in Python bindings)
      * \returns calculated aggregate value
      * \since QGIS 2.16
      */
@@ -2265,7 +2266,8 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
                         QgsExpressionContext *context = nullptr,
                         bool *ok = nullptr,
                         QgsFeatureIds *fids = nullptr,
-                        QgsFeedback *feedback = nullptr ) const;
+                        QgsFeedback *feedback = nullptr,
+                        QString *error SIP_PYARGREMOVE = nullptr ) const;
 
     //! Sets the blending mode used for rendering each feature
     void setFeatureBlendMode( QPainter::CompositionMode blendMode );