Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 23cbeda

Browse files
committed
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
1 parent 03109a5 commit 23cbeda

File tree

2 files changed

+19
-20
lines changed

2 files changed

+19
-20
lines changed

src/backend/utils/adt/numeric.c

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5172,21 +5172,35 @@ numeric_stddev_internal(NumericAggState *state,
51725172
vsumX,
51735173
vsumX2,
51745174
vNminus1;
5175-
const NumericVar *comp;
5175+
int64 totCount;
51765176
int rscale;
51775177

5178-
/* Deal with empty input and NaN-input cases */
5179-
if (state == NULL || (state->N + state->NaNcount) == 0)
5178+
/*
5179+
* Sample stddev and variance are undefined when N <= 1; population stddev
5180+
* is undefined when N == 0. Return NULL in either case (note that NaNs
5181+
* count as normal inputs for this purpose).
5182+
*/
5183+
if (state == NULL || (totCount = state->N + state->NaNcount) == 0)
5184+
{
5185+
*is_null = true;
5186+
return NULL;
5187+
}
5188+
5189+
if (sample && totCount <= 1)
51805190
{
51815191
*is_null = true;
51825192
return NULL;
51835193
}
51845194

51855195
*is_null = false;
51865196

5197+
/*
5198+
* Deal with NaN inputs.
5199+
*/
51875200
if (state->NaNcount > 0)
51885201
return make_result(&const_nan);
51895202

5203+
/* OK, normal calculation applies */
51905204
init_var(&vN);
51915205
init_var(&vsumX);
51925206
init_var(&vsumX2);
@@ -5195,21 +5209,6 @@ numeric_stddev_internal(NumericAggState *state,
51955209
accum_sum_final(&(state->sumX), &vsumX);
51965210
accum_sum_final(&(state->sumX2), &vsumX2);
51975211

5198-
/*
5199-
* Sample stddev and variance are undefined when N <= 1; population stddev
5200-
* is undefined when N == 0. Return NULL in either case.
5201-
*/
5202-
if (sample)
5203-
comp = &const_one;
5204-
else
5205-
comp = &const_zero;
5206-
5207-
if (cmp_var(&vN, comp) <= 0)
5208-
{
5209-
*is_null = true;
5210-
return NULL;
5211-
}
5212-
52135212
init_var(&vNminus1);
52145213
sub_var(&vN, &const_one, &vNminus1);
52155214

src/test/regress/expected/aggregates.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,13 @@ SELECT stddev_pop(3.0::numeric), stddev_samp(4.0::numeric);
214214
SELECT var_pop('nan'::numeric), var_samp('nan'::numeric);
215215
var_pop | var_samp
216216
---------+----------
217-
NaN | NaN
217+
NaN |
218218
(1 row)
219219

220220
SELECT stddev_pop('nan'::numeric), stddev_samp('nan'::numeric);
221221
stddev_pop | stddev_samp
222222
------------+-------------
223-
NaN | NaN
223+
NaN |
224224
(1 row)
225225

226226
-- verify correct results for null and NaN inputs

0 commit comments

Comments
 (0)