At Tue, 07 May 2019 20:47:28 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote in <20190507.204728.233299873.horiguchi.kyot...@lab.ntt.co.jp> > Hello. > > At Tue, 7 May 2019 14:39:55 +0530, Rajkumar Raghuwanshi > <rajkumar.raghuwan...@enterprisedb.com> wrote in > <CAKcux6=ybmcntcafss_22ds1ab6mgay_quahx-nvg+_fvpm...@mail.gmail.com> > > Hi, > > As this issue is reproducible without partition-wise aggregate also, > > changing email subject from "Statistical aggregate functions are not > > working with partitionwise aggregate " to "Statistical aggregate functions > > are not working with PARTIAL aggregation". > > > > original reported test case and discussion can be found at below link. > > https://www.postgresql.org/message-id/flat/CAKcux6%3DuZEyWyLw0N7HtR9OBc-sWEFeByEZC7t-KDf15FKxVew%40mail.gmail.com > > The immediate reason for the behavior seems that > EEOP_AGG_STRICT_INPUT_CHECK_ARGS considers non existent second > argument as null, which is out of arguments list in > trans_fcinfo->args[]. > > The invalid deserialfn_oid case in ExecBuildAggTrans, it > initializes args[1] using the second argument of the functoin > (int8pl() in the case) so the correct numTransInputs here is 1, > not 2. > > I don't understand this fully but at least the attached patch > makes the test case work correctly and this seems to be the only > case of this issue.
This behavior is introduced by 69c3936a14 (in v11). At that time FunctionCallInfoData is pallioc0'ed and has fixed length members arg[6] and argnull[7]. So nulls[1] is always false even if nargs = 1 so the issue had not been revealed. After introducing a9c35cf85c (in v12) the same check is done on FunctionCallInfoData that has NullableDatum args[] of required number of elements. In that case args[1] is out of palloc'ed memory so this issue has been revealed. In a second look, I seems to me that the right thing to do here is setting numInputs instaed of numArguments to numTransInputs in combining step. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
>From c5dcbc32646e8e4865307bb0525a143da573e240 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Wed, 8 May 2019 13:01:01 +0900 Subject: [PATCH] Give correct number of arguments to combine function. Combine function's first parameter is used as transition value. The correct number of transition input for the function is not the number of arguments but of transition input. --- src/backend/executor/nodeAgg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index d01fc4f52e..e13a8cb304 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2912,7 +2912,8 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, pertrans->aggtranstype = aggtranstype; /* Detect how many arguments to pass to the transfn */ - if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) + if (AGGKIND_IS_ORDERED_SET(aggref->aggkind) || + DO_AGGSPLIT_COMBINE(aggstate->aggsplit)) pertrans->numTransInputs = numInputs; else pertrans->numTransInputs = numArguments; -- 2.16.3
