|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>|
|Cc:||guofenglinux(at)gmail(dot)com, andrew(at)tao11(dot)riddles(dot)org(dot)uk, michael(at)paquier(dot)xyz, cyg0810(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org|
|Subject:||Re: BUG #17088: FailedAssertion in prepagg.c|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> writes:
> At Fri, 9 Jul 2021 14:54:02 +0800, Richard Guo <guofenglinux(at)gmail(dot)com> wrote in
>> Update the patch with comments and test cases.
> AFAICS the patch looks correct. It works for the first example and
> the two from Tom. I don't find other place that has the similar
I'd been expecting Andrew to pick this up, but since he hasn't,
I took a look.
I concur that the core problem is that GroupingFunc has to be treated
almost exactly like Aggref, and here we have a couple of places that
didn't get that memo. So it occurred to me to look for other places
that special-case Aggref and don't have parallel code for GroupingFunc,
and I found several:
This isn't particularly hazardous, since the argument (probably?) can't
contain a SRF, but it still seems like it ought to treat the two node
types the same.
It's defaulting to charging the eval costs of the arguments, which is
flat wrong. I made it charge one cpu_operator_cost instead.
Various places concerned with whether or not we need parens were
making the wrong choice, resulting in excess parens in pretty-printing
mode. This is also just cosmetic, but still.
This looks good to me now, and I'll set about back-patching.
regards, tom lane
|Next Message||David G. Johnston||2022-03-21 22:23:30||Re: BUG #17445: "ON CONFLICT" has different behaviors when its param is passed with prepared stmt or hard coded|
|Previous Message||PG Bug reporting form||2022-03-21 20:37:15||BUG #17445: "ON CONFLICT" has different behaviors when its param is passed with prepared stmt or hard coded|