From bf6af3538268a10dd972681b546687b6640cac8e Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 9 Aug 2021 10:08:04 +1000 Subject: [PATCH] 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 --- .../auto_generated/qgsaggregatecalculator.sip.in | 9 ++++++++- .../auto_generated/vector/qgsvectorlayer.sip.in | 3 ++- src/core/expression/qgsexpressionfunction.cpp | 10 +++++++--- src/core/qgsaggregatecalculator.cpp | 4 ++++ src/core/qgsaggregatecalculator.h | 11 ++++++++++- src/core/vector/qgsvectorlayer.cpp | 15 +++++++++++++-- src/core/vector/qgsvectorlayer.h | 4 +++- 7 files changed, 47 insertions(+), 9 deletions(-) diff --git a/python/core/auto_generated/qgsaggregatecalculator.sip.in b/python/core/auto_generated/qgsaggregatecalculator.sip.in index 95aeb96dd70..97c0f3505a3 100644 --- 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 index 5846648b919..d84b36306e6 100644 --- 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 index f584340a3de..c5ffa30c22d 100644 --- 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 index d604b05afca..a3464ff7f5b 100644 --- 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 lst; diff --git a/src/core/qgsaggregatecalculator.h b/src/core/qgsaggregatecalculator.h index a4ec8ad0f39..5b606fff244 100644 --- 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 index 03ed37cf5bf..60a5f857e90 100644 --- 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 ¶meters, 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 index 98d71ae1422..42a16680637 100644 --- 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 );