database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-06 19:59:18 +03:00
Code

Correctly set up aggregate FILTER expression in partial-aggregation plans.

Browse Source 
The aggfilter expression should be removed from the parent (combining)
Aggref, since it's not supposed to apply the filter, and indeed cannot
because any Vars used in the filter would not be available after the
lower-level aggregation step.  Per report from Jeff Janes.

(This has been broken since the introduction of partial aggregation,
I think.  The error became obvious after commit 59a3795c2, when setrefs.c
began processing the parent Aggref's fields normally and thus would detect
such Vars.  The special-case coding previously used in setrefs.c skipped
over the parent's aggfilter field without processing it.  That was broken
in its own way because no other setrefs.c processing got applied either;
though since the executor would not execute the filter expression, only
initialize it, that oversight might not have had any visible symptoms at
present.)

Report: <CAMkU=1xfuPf2edAe4ZGXTmJpU7jxuKukKyvNtEXwu35B7dvejg@mail.gmail.com>
This commit is contained in:
Tom Lane 2016-07-23 20:16:48 -04:00
parent 9d7abca901
commit 6d85bb1ba7
1 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/backend/optimizer/plan/setrefs.c
View File

@ -1752,6 +1752,10 @@ convert_combining_aggrefs(Node *node, void *context)
Aggref *child_agg; Aggref *child_agg;
Aggref *parent_agg; Aggref *parent_agg;
/* Assert we've not chosen to partial-ize any unsupported cases */
Assert(orig_agg->aggorder == NIL);
Assert(orig_agg->aggdistinct == NIL);
/* /*
* Since aggregate calls can't be nested, we needn't recurse into the * Since aggregate calls can't be nested, we needn't recurse into the
* arguments. But for safety, flat-copy the Aggref node itself rather * arguments. But for safety, flat-copy the Aggref node itself rather
@ -1762,13 +1766,17 @@ convert_combining_aggrefs(Node *node, void *context)
/* /*
* For the parent Aggref, we want to copy all the fields of the * For the parent Aggref, we want to copy all the fields of the
* original aggregate *except* the args list. Rather than explicitly * original aggregate *except* the args list, which we'll replace
* knowing what they all are here, we can momentarily modify child_agg * below, and the aggfilter expression, which should be applied only
* to provide a source for copyObject. * by the child not the parent. Rather than explicitly knowing about
* all the other fields here, we can momentarily modify child_agg to
* provide a suitable source for copyObject.
*/ */
child_agg->args = NIL; child_agg->args = NIL;
child_agg->aggfilter = NULL;
parent_agg = (Aggref *) copyObject(child_agg); parent_agg = (Aggref *) copyObject(child_agg);
child_agg->args = orig_agg->args; child_agg->args = orig_agg->args;
child_agg->aggfilter = orig_agg->aggfilter;
/* /*
* Now, set up child_agg to represent the first phase of partial * Now, set up child_agg to represent the first phase of partial