[aggregate calculator] Improve result type detection to fix bogus typ…

…eless scenarios
qgis · Aug 17, 2021 · 36b8b31 · 36b8b31
1 parent 242aa98
commit 36b8b31
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 5 deletions.
diff --git a/src/core/qgsaggregatecalculator.cpp b/src/core/qgsaggregatecalculator.cpp
@@ -111,7 +111,7 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
     {
       // evaluate first feature, check result type
       QgsFeatureRequest testRequest( request );
-      testRequest.setLimit( 1 );
+      testRequest.setLimit( 10 );
       QgsFeature f;
       QgsFeatureIterator fit = mLayer->getFeatures( testRequest );
       if ( !fit.nextFeature( f ) )
@@ -122,10 +122,70 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
         return defaultValue( aggregate );
       }
 
-      if ( context )
-        context->setFeature( f );
-      QVariant v = expression->evaluate( context );
-      resultType = v.type();
+      bool hasFeature = true;
+      bool foundType = false;
+      while ( hasFeature && !foundType )
+      {
+        if ( context )
+          context->setFeature( f );
+        QVariant v = expression->evaluate( context );
+        if ( !v.isNull() )
+        {
+          resultType = v.type();
+          foundType = true;
+        }
+        else
+        {
+          hasFeature = fit.nextFeature( f );
+        }
+      }
+
+      if ( !foundType )
+      {
+        QVariant v;
+        switch ( aggregate )
+        {
+          // string
+          case StringConcatenate:
+          case StringConcatenateUnique:
+          case StringMinimumLength:
+          case StringMaximumLength:
+            v = QString();
+            break;
+
+          // numerical
+          case Sum:
+          case Mean:
+          case Median:
+          case StDev:
+          case StDevSample:
+          case Range:
+          case FirstQuartile:
+          case ThirdQuartile:
+          case InterQuartileRange:
+          // mixed type, fallback to numerical
+          case Count:
+          case CountDistinct:
+          case CountMissing:
+          case Minority:
+          case Majority:
+          case Min:
+          case Max:
+            v = 0.0;
+            break;
+
+          // geometry
+          case GeometryCollect:
+            v = QgsGeometry();
+            break;
+
+          // list, fallback to string
+          case ArrayAggregate:
+            v = QString();
+            break;
+        }
+        resultType = v.type();
+      }
     }
   }
   else

diff --git a/tests/src/python/test_qgsaggregatecalculator.py b/tests/src/python/test_qgsaggregatecalculator.py
@@ -431,6 +431,35 @@ def testExpression(self):
         self.assertTrue(ok)
         self.assertEqual(val, 0.0)
 
+    def testExpressionNullValuesAtStart(self):
+        """ test aggregate calculation using an expression which returns null values at first """
+
+        # numeric
+        layer = QgsVectorLayer("Point?field=fldstr:string", "layer", "memory")
+        pr = layer.dataProvider()
+
+        values = [None, None, None, None, None, None, None, None, None, None, '2', '3', '5']
+
+        features = []
+        for v in values:
+            f = QgsFeature()
+            f.setFields(layer.fields())
+            f.setAttributes([v])
+            features.append(f)
+        assert pr.addFeatures(features)
+
+        # number aggregation
+        agg = QgsAggregateCalculator(layer)
+        val, ok = agg.calculate(QgsAggregateCalculator.Sum, 'to_int(fldstr)')
+        self.assertTrue(ok)
+        self.assertEqual(val, 10)
+
+        # string aggregation
+        agg.setDelimiter(',')
+        val, ok = agg.calculate(QgsAggregateCalculator.StringConcatenate, 'fldstr || \'suffix\'')
+        self.assertTrue(ok)
+        self.assertEqual(val, ',,,,,,,,,,2suffix,3suffix,5suffix')
+
     def testExpressionNoMatch(self):
         """ test aggregate calculation using an expression with no features """