Skip to content

Commit

Permalink
Fix calculation of certain aggregates from expressions when no
Browse files Browse the repository at this point in the history
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.
38 changes: 37 additions & 1 deletion src/core/qgsaggregatecalculator.cpp
Expand Up @@ -101,7 +101,7 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
//no matching features
if ( ok )
*ok = true;
return QVariant();
return defaultValue( aggregate );
}

if ( context )
Expand Down Expand Up @@ -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 )
{
Expand Down
2 changes: 2 additions & 0 deletions src/core/qgsaggregatecalculator.h
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions tests/src/python/test_qgsaggregatecalculator.py
Expand Up @@ -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 """

Expand Down

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.