pgsql: Fix bugs with degenerate window ORDER BY clauses inGROUPS/RANGE - Mailing list pgsql-committers

Fix bugs with degenerate window ORDER BY clauses in GROUPS/RANGE mode.

nodeWindowAgg.c failed to cope with the possibility that no ordering
columns are defined in the window frame for GROUPS mode or RANGE OFFSET
mode, leading to assertion failures or odd errors, as reported by Masahiko
Sawada and Lukas Eder.  In RANGE OFFSET mode, an ordering column is really
required, so add an Assert about that.  In GROUPS mode, the code would
work, except that the node initialization code wasn't in sync with the
execution code about when to set up tuplestore read pointers and spare
slots.  Fix the latter for consistency's sake (even though I think the
changes described below make the out-of-sync cases unreachable for now).

Per SQL spec, a single ordering column is required for RANGE OFFSET mode,
and at least one ordering column is required for GROUPS mode.  The parser
enforced the former but not the latter; add a check for that.

We were able to reach the no-ordering-column cases even with fully spec
compliant queries, though, because the planner would drop partitioning
and ordering columns from the generated plan if they were redundant with
earlier columns according to the redundant-pathkey logic, for instance
"PARTITION BY x ORDER BY y" in the presence of a "WHERE x=y" qual.
While in principle that's an optimization that could save some pointless
comparisons at runtime, it seems unlikely to be meaningful in the real
world.  I think this behavior was not so much an intentional optimization
as a side-effect of an ancient decision to construct the plan node's
ordering-column info by reverse-engineering the PathKeys of the input
path.  If we give up redundant-column removal then it takes very little
code to generate the plan node info directly from the WindowClause,
ensuring that we have the expected number of ordering columns in all
cases.  (If anyone does complain about this, the planner could perhaps
be taught to remove redundant columns only when it's safe to do so,
ie *not* in RANGE OFFSET mode.  But I doubt anyone ever will.)

With these changes, the WindowAggPath.winpathkeys field is not used for
anything anymore, so remove it.

The test cases added here are not actually very interesting given the
removal of the redundant-column-removal logic, but they would represent
important corner cases if anyone ever tries to put that back.

Tom Lane and Masahiko Sawada.  Back-patch to v11 where RANGE OFFSET
and GROUPS modes were added.

Discussion: https://postgr.es/m/CAD21AoDrWqycq-w_+Bx1cjc+YUhZ11XTj9rfxNiNDojjBx8Fjw@mail.gmail.com
Discussion: https://postgr.es/m/153086788677.17476.8002640580496698831@wrigleys.postgresql.org

Branch
------
REL_11_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/8893d48e7fbfacb77fee0080faf7b4228c33ce35

Modified Files
--------------
src/backend/executor/nodeWindowAgg.c    |  45 +++++---
src/backend/nodes/outfuncs.c            |   1 -
src/backend/optimizer/plan/createplan.c | 184 +++++++-------------------------
src/backend/optimizer/plan/planner.c    |   5 +-
src/backend/optimizer/util/pathnode.c   |   9 +-
src/backend/parser/parse_clause.c       |  10 ++
src/include/nodes/relation.h            |   5 -
src/include/optimizer/pathnode.h        |   3 +-
src/test/regress/expected/window.out    |  95 +++++++++++++++++
src/test/regress/sql/window.sql         |  38 +++++++
10 files changed, 217 insertions(+), 178 deletions(-)