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

Commit 9ccae63

Browse files
committed
Repair corner-case bug in array version of percentile_cont().
The code for advancing through the input rows overlooked the case that we might already be past the first row of the row pair now being considered, in case the previous percentile also fell between the same two input rows. Report and patch by Andrew Gierth; logic rewritten a bit for clarity by me.
1 parent 1a1fb46 commit 9ccae63

File tree

3 files changed

+26
-18
lines changed

3 files changed

+26
-18
lines changed

src/backend/utils/adt/orderedsetaggs.c

+21-13
Original file line numberDiff line numberDiff line change
@@ -904,42 +904,50 @@ percentile_cont_multi_final_common(FunctionCallInfo fcinfo,
904904

905905
for (; i < num_percentiles; i++)
906906
{
907-
int64 target_row = pct_info[i].first_row;
908-
bool need_lerp = (pct_info[i].second_row > target_row);
907+
int64 first_row = pct_info[i].first_row;
908+
int64 second_row = pct_info[i].second_row;
909909
int idx = pct_info[i].idx;
910910

911-
/* Advance to first_row, if not already there */
912-
if (target_row > rownum)
911+
/*
912+
* Advance to first_row, if not already there. Note that we might
913+
* already have rownum beyond first_row, in which case first_val
914+
* is already correct. (This occurs when interpolating between
915+
* the same two input rows as for the previous percentile.)
916+
*/
917+
if (first_row > rownum)
913918
{
914-
if (!tuplesort_skiptuples(osastate->sortstate, target_row - rownum - 1, true))
919+
if (!tuplesort_skiptuples(osastate->sortstate, first_row - rownum - 1, true))
915920
elog(ERROR, "missing row in percentile_cont");
916921

917922
if (!tuplesort_getdatum(osastate->sortstate, true, &first_val, &isnull) || isnull)
918923
elog(ERROR, "missing row in percentile_cont");
919924

920-
rownum = target_row;
925+
rownum = first_row;
926+
/* Always advance second_val to be latest input value */
927+
second_val = first_val;
921928
}
922-
else
929+
else if (first_row == rownum)
923930
{
924931
/*
925-
* We are already at the target row, so we must previously
926-
* have read its value into second_val.
932+
* We are already at the desired row, so we must previously
933+
* have read its value into second_val (and perhaps first_val
934+
* as well, but this assignment is harmless in that case).
927935
*/
928936
first_val = second_val;
929937
}
930938

931939
/* Fetch second_row if needed */
932-
if (need_lerp)
940+
if (second_row > rownum)
933941
{
934942
if (!tuplesort_getdatum(osastate->sortstate, true, &second_val, &isnull) || isnull)
935943
elog(ERROR, "missing row in percentile_cont");
936944
rownum++;
937945
}
938-
else
939-
second_val = first_val;
946+
/* We should now certainly be on second_row exactly */
947+
Assert(second_row == rownum);
940948

941949
/* Compute appropriate result */
942-
if (need_lerp)
950+
if (second_row > first_row)
943951
result_datum[idx] = lerpfunc(first_val, second_val,
944952
pct_info[i].proportion);
945953
else

src/test/regress/expected/aggregates.out

+4-4
Original file line numberDiff line numberDiff line change
@@ -1423,11 +1423,11 @@ from tenk1;
14231423
{{NULL,999,499},{749,249,NULL}}
14241424
(1 row)
14251425

1426-
select percentile_cont(array[0,1,0.25,0.75,0.5,1]) within group (order by x)
1426+
select percentile_cont(array[0,1,0.25,0.75,0.5,1,0.3,0.32,0.35,0.38,0.4]) within group (order by x)
14271427
from generate_series(1,6) x;
1428-
percentile_cont
1429-
-----------------------
1430-
{1,6,2.25,4.75,3.5,6}
1428+
percentile_cont
1429+
------------------------------------------
1430+
{1,6,2.25,4.75,3.5,6,2.5,2.6,2.75,2.9,3}
14311431
(1 row)
14321432

14331433
select ten, mode() within group (order by string4) from tenk1 group by ten;

src/test/regress/sql/aggregates.sql

+1-1
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ select percentile_cont(array[0,0.25,0.5,0.75,1]) within group (order by thousand
533533
from tenk1;
534534
select percentile_disc(array[[null,1,0.5],[0.75,0.25,null]]) within group (order by thousand)
535535
from tenk1;
536-
select percentile_cont(array[0,1,0.25,0.75,0.5,1]) within group (order by x)
536+
select percentile_cont(array[0,1,0.25,0.75,0.5,1,0.3,0.32,0.35,0.38,0.4]) within group (order by x)
537537
from generate_series(1,6) x;
538538

539539
select ten, mode() within group (order by string4) from tenk1 group by ten;

0 commit comments

Comments
 (0)