pgsql: Fix window functions that sort by expressions involving aggregat

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Fix window functions that sort by expressions involving aggregat
Date: 2011-09-27 03:58:59
Message-ID: E1R8Oot-00022Q-IH@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix window functions that sort by expressions involving aggregates.

In commit c1d9579dd8bf3c921ca6bc2b62c40da6d25372e5, I changed things so
that the output of the Agg node that feeds the window functions would not
list any ungrouped Vars directly. Formerly, for example, the Agg tlist
might have included both "x" and "sum(x)", which is not really valid if
"x" isn't a grouping column. If we then had a window function ordering on
something like "sum(x) + 1", prepare_sort_from_pathkeys would find no exact
match for this in the Agg tlist, and would conclude that it must recompute
the expression. But it would break the expression down to just the Var
"x", which it would find in the tlist, and then rebuild the ORDER BY
expression using a reference to the subplan's "x" output. Now, after the
above-referenced changes, "x" isn't in the Agg tlist if it's not a grouping
column, so that prepare_sort_from_pathkeys fails with "could not find
pathkey item to sort", as reported by Bricklen Anderson.

The fix is to not break down Aggrefs into their component parts, but just
treat them as irreducible expressions to be sought in the subplan tlist.
This is definitely OK for the use with respect to window functions in
grouping_planner, since it just built the tlist being used on the same
basis. AFAICT it is safe for other uses too; most of the other call sites
couldn't encounter Aggrefs anyway.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/269c5dd2f46e3490da05d5dd5dad07828df281d9

Modified Files
--------------
src/backend/optimizer/plan/createplan.c | 10 ++++++++--
src/test/regress/expected/window.out | 20 ++++++++++++++++++++
src/test/regress/sql/window.sql | 7 +++++++
3 files changed, 35 insertions(+), 2 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Robert Haas 2011-09-27 12:24:34 pgsql: heap_update() must recheck tuple after unlocking and relocking b
Previous Message Tom Lane 2011-09-27 02:25:59 pgsql: Allow snapshot references to still work during transaction abort