Skip site navigation (1) Skip section navigation (2)

pgsql: Centralize the logic for detecting misplaced aggregates,window

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Centralize the logic for detecting misplaced aggregates,window
Date: 2012-08-10 15:36:50
Message-ID: E1SzrGc-0004ot-4q@gemulon.postgresql.org (view raw or flat)
Thread:
Lists: pgsql-committers
Centralize the logic for detecting misplaced aggregates, window funcs, etc.

Formerly we relied on checking after-the-fact to see if an expression
contained aggregates, window functions, or sub-selects when it shouldn't.
This is grotty, easily forgotten (indeed, we had forgotten to teach
DefineIndex about rejecting window functions), and none too efficient
since it requires extra traversals of the parse tree.  To improve matters,
define an enum type that classifies all SQL sub-expressions, store it in
ParseState to show what kind of expression we are currently parsing, and
make transformAggregateCall, transformWindowFuncCall, and transformSubLink
check the expression type and throw error if the type indicates the
construct is disallowed.  This allows removal of a large number of ad-hoc
checks scattered around the code base.  The enum type is sufficiently
fine-grained that we can still produce error messages of at least the
same specificity as before.

Bringing these error checks together revealed that we'd been none too
consistent about phrasing of the error messages, so standardize the wording
a bit.

Also, rewrite checking of aggregate arguments so that it requires only one
traversal of the arguments, rather than up to three as before.

In passing, clean up some more comments left over from add_missing_from
support, and annotate some tests that I think are dead code now that that's
gone.  (I didn't risk actually removing said dead code, though.)

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/eaccfded98a9c677d3a2e849c1747ec90e8596a6

Modified Files
--------------
src/backend/catalog/heap.c               |   45 +--
src/backend/commands/functioncmds.c      |   24 +-
src/backend/commands/indexcmds.c         |   26 +--
src/backend/commands/prepare.c           |   16 +-
src/backend/commands/tablecmds.c         |   17 +-
src/backend/commands/trigger.c           |   17 +-
src/backend/commands/typecmds.c          |   33 +--
src/backend/optimizer/util/clauses.c     |    2 +-
src/backend/optimizer/util/var.c         |  161 ---------
src/backend/parser/analyze.c             |  138 ++------
src/backend/parser/parse_agg.c           |  540 +++++++++++++++++++++---------
src/backend/parser/parse_clause.c        |  202 +++++++-----
src/backend/parser/parse_expr.c          |  274 +++++++++++++---
src/backend/parser/parse_node.c          |    4 +-
src/backend/parser/parse_target.c        |   77 +++--
src/backend/parser/parse_utilcmd.c       |   29 +-
src/backend/rewrite/rewriteManip.c       |   17 +-
src/include/optimizer/var.h              |    1 -
src/include/parser/parse_agg.h           |    1 -
src/include/parser/parse_clause.h        |    9 +-
src/include/parser/parse_expr.h          |    4 +-
src/include/parser/parse_node.h          |   49 +++
src/include/parser/parse_target.h        |    9 +-
src/include/rewrite/rewriteManip.h       |    3 +-
src/test/regress/expected/aggregates.out |   11 +-
src/test/regress/expected/join.out       |    2 +-
src/test/regress/expected/window.out     |   14 +-
src/test/regress/expected/with.out       |    4 +-
src/test/regress/sql/aggregates.sql      |    4 +
29 files changed, 952 insertions(+), 781 deletions(-)

pgsql-committers by date

Next:From: Bruce MomjianDate: 2012-08-10 18:11:02
Subject: pgsql: Fix pgtest to return proper error code based on 'make' returnco
Previous:From: Magnus HaganderDate: 2012-08-10 12:57:37
Subject: pgsql: Fix upper limit of superuser_reserved_connections,add limit for

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group