postgres.git / summary / log / commit / refs

commit    23cbeda50b94c817bed4f7d2127ee09c4e8c8b86
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sat Jun 13 18:01:46 2020 +0000
First released in: 14.0

    Sync behavior of var_samp and stddev_samp for single NaN inputs.
    
    var_samp(numeric) and stddev_samp(numeric) disagreed with their float
    cousins about what to do for a single non-null input value that is NaN.
    The float versions return NULL on the grounds that the calculation is
    only defined for more than one non-null input, which seems like the
    right answer.  But the numeric versions returned NaN, as a result of
    dealing with edge cases in the wrong order.  Fix that.  The patch
    also gets rid of an insignificant memory leak in such cases.
    
    This inconsistency is of long standing, but on the whole it seems best
    not to back-patch the change into stable branches; nobody's complained
    and it's such an obscure point that nobody's likely to complain.
    (Note that v13 and v12 now contain test cases that will notice if we
    accidentally back-patch this behavior change in future.)
    
    Report and patch by me; thanks to Dean Rasheed for review.
    
    Discussion: https://postgr.es/m/353062.1591898766@sss.pgh.pa.us


 src/backend/utils/adt/numeric.c          | 35 ++++++++++++++++----------------
 src/test/regress/expected/aggregates.out |  4 ++--
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 553e261ed00..eea42398541 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -5172,11 +5172,21 @@ numeric_stddev_internal(NumericAggState *state,
 				vsumX,
 				vsumX2,
 				vNminus1;
-	const NumericVar *comp;
+	int64		totCount;
 	int			rscale;
 
-	/* Deal with empty input and NaN-input cases */
-	if (state == NULL || (state->N + state->NaNcount) == 0)
+	/*
+	 * Sample stddev and variance are undefined when N <= 1; population stddev
+	 * is undefined when N == 0.  Return NULL in either case (note that NaNs
+	 * count as normal inputs for this purpose).
+	 */
+	if (state == NULL || (totCount = state->N + state->NaNcount) == 0)
+	{
+		*is_null = true;
+		return NULL;
+	}
+
+	if (sample && totCount <= 1)
 	{
 		*is_null = true;
 		return NULL;
@@ -5184,9 +5194,13 @@ numeric_stddev_internal(NumericAggState *state,
 
 	*is_null = false;
 
+	/*
+	 * Deal with NaN inputs.
+	 */
 	if (state->NaNcount > 0)
 		return make_result(&const_nan);
 
+	/* OK, normal calculation applies */
 	init_var(&vN);
 	init_var(&vsumX);
 	init_var(&vsumX2);
@@ -5195,21 +5209,6 @@ numeric_stddev_internal(NumericAggState *state,
 	accum_sum_final(&(state->sumX), &vsumX);
 	accum_sum_final(&(state->sumX2), &vsumX2);
 
-	/*
-	 * Sample stddev and variance are undefined when N <= 1; population stddev
-	 * is undefined when N == 0. Return NULL in either case.
-	 */
-	if (sample)
-		comp = &const_one;
-	else
-		comp = &const_zero;
-
-	if (cmp_var(&vN, comp) <= 0)
-	{
-		*is_null = true;
-		return NULL;
-	}
-
 	init_var(&vNminus1);
 	sub_var(&vN, &const_one, &vNminus1);
 
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index e4ffa5ee426..3bd184ae294 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -214,13 +214,13 @@ SELECT stddev_pop(3.0::numeric), stddev_samp(4.0::numeric);
 SELECT var_pop('nan'::numeric), var_samp('nan'::numeric);
  var_pop | var_samp 
 ---------+----------
-     NaN |      NaN
+     NaN |         
 (1 row)
 
 SELECT stddev_pop('nan'::numeric), stddev_samp('nan'::numeric);
  stddev_pop | stddev_samp 
 ------------+-------------
-        NaN |         NaN
+        NaN |            
 (1 row)
 
 -- verify correct results for null and NaN inputs


[parent: 03109a53020e]