Please sign in to comment.
Avoid listing ungrouped Vars in the targetlist of Agg-underneath-Window.
Regular aggregate functions in combination with, or within the arguments of, window functions are OK per spec; they have the semantics that the aggregate output rows are computed and then we run the window functions over that row set. (Thus, this combination is not really useful unless there's a GROUP BY so that more than one aggregate output row is possible.) The case without GROUP BY could fail, as recently reported by Jeff Davis, because sloppy construction of the Agg node's targetlist resulted in extra references to possibly-ungrouped Vars appearing outside the aggregate function calls themselves. See the added regression test case for an example. Fixing this requires modifying the API of flatten_tlist and its underlying function pull_var_clause. I chose to make pull_var_clause's API for aggregates identical to what it was already doing for placeholders, since the useful behaviors turn out to be the same (error, report node as-is, or recurse into it). I also tightened the error checking in this area a bit: if it was ever valid to see an uplevel Var, Aggref, or PlaceHolderVar here, that was a long time ago, so complain instead of ignoring them. Backpatch into 9.1. The failure exists in 8.4 and 9.0 as well, but seeing that it only occurs in a basically-useless corner case, it doesn't seem worth the risks of changing a function API in a minor release. There might be third-party code using pull_var_clause.
- Loading branch information...
Showing with 119 additions and 78 deletions.
- +6 −2 src/backend/catalog/heap.c
- +3 −1 src/backend/commands/trigger.c
- +5 −2 src/backend/optimizer/path/allpaths.c
- +1 −0 src/backend/optimizer/path/equivclass.c
- +1 −0 src/backend/optimizer/plan/createplan.c
- +4 −1 src/backend/optimizer/plan/initsplan.c
- +20 −11 src/backend/optimizer/plan/planner.c
- +1 −0 src/backend/optimizer/prep/preptlist.c
- +0 −36 src/backend/optimizer/util/clauses.c
- +4 −1 src/backend/optimizer/util/placeholder.c
- +6 −5 src/backend/optimizer/util/tlist.c
- +42 −13 src/backend/optimizer/util/var.c
- +3 −1 src/backend/utils/adt/selfuncs.c
- +0 −1 src/include/optimizer/clauses.h
- +3 −2 src/include/optimizer/tlist.h
- +10 −2 src/include/optimizer/var.h
- +7 −0 src/test/regress/expected/window.out
- +3 −0 src/test/regress/sql/window.sql
Oops, something went wrong.