Skip to content
Permalink
Browse files
Fix calculation of certain aggregates from expressions when no
matching features exist

Eg sum and count should return 0 in this case rather than
null
  • Loading branch information
nyalldawson committed Aug 29, 2016
1 parent 420311e commit 9ba41e9
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
@@ -101,7 +101,7 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
//no matching features
if ( ok )
*ok = true;
return QVariant();
return defaultValue( aggregate );
}

if ( context )
@@ -497,6 +497,42 @@ QVariant QgsAggregateCalculator::concatenateStrings( QgsFeatureIterator& fit, in
return result;
}

QVariant QgsAggregateCalculator::defaultValue( QgsAggregateCalculator::Aggregate aggregate ) const
{
// value to return when NO features are aggregated:
switch ( aggregate )
{
// sensible values:
case Count:
case CountDistinct:
case CountMissing:
case Sum:
return 0;

case StringConcatenate:
return ""; // zero length string - not null!

// undefined - nothing makes sense here
case Min:
case Max:
case Mean:
case Median:
case StDev:
case StDevSample:
case Range:
case Minority:
case Majority:
case FirstQuartile:
case ThirdQuartile:
case InterQuartileRange:
case StringMinimumLength:
case StringMaximumLength:
case GeometryCollect:
return QVariant();
}
return QVariant();
}

QVariant QgsAggregateCalculator::calculateDateTimeAggregate( QgsFeatureIterator& fit, int attr, QgsExpression* expression,
QgsExpressionContext* context, QgsDateTimeStatisticalSummary::Statistic stat )
{
@@ -169,6 +169,8 @@ class CORE_EXPORT QgsAggregateCalculator
QgsExpressionContext* context, bool* ok = nullptr );
static QVariant concatenateStrings( QgsFeatureIterator& fit, int attr, QgsExpression* expression,
QgsExpressionContext* context, const QString& delimiter );

QVariant defaultValue( Aggregate aggregate ) const;
};

#endif //QGSAGGREGATECALCULATOR_H
@@ -386,6 +386,36 @@ def testExpression(self):
self.assertTrue(ok)
self.assertEqual(val, 5)

def testExpressionNoMatch(self):
""" test aggregate calculation using an expression with no features """

# no features
layer = QgsVectorLayer("Point?field=fldint:integer", "layer", "memory")

# sum
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.Sum, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, 0)

# count
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.Count, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, 0)

# count distinct
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.CountDistinct, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, 0)

# count missing
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.CountMissing, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, 0)

def testStringToAggregate(self):
""" test converting strings to aggregate types """

1 comment on commit 9ba41e9

@nirvn
Copy link
Contributor

@nirvn nirvn commented on 9ba41e9 Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson , brilliant, thanks.

Going back to battleship now... ;)

Please sign in to comment.