Re: Row pattern recognition

Hi Jian, Tatsuo,

While auditing DEFINE clause behavior against the parse tree, I found a
case where RPR rejects a query that is valid on plain PostgreSQL, plus a
related case where a view using the same construct produces a deparse
output that doesn't round-trip. Here's how the gap arises, in order.

Trigger: a FULL JOIN's USING merged column, referenced from GROUP BY
using its expanded spelling (a COALESCE of both sides) rather than its
bare join name, while DEFINE also reads that column.

Setup:

CREATE TABLE g1 (id int primary key, cat text, val int);
CREATE TABLE g2 (id int, cat text, v2 int);
INSERT INTO g1 SELECT i, 'c'||(i%3), i*10 FROM generate_series(1,9) i;
INSERT INTO g2 SELECT i, 'c'||(i%3), i FROM generate_series(1,9) i;

Background -- why "id" is equivalent to COALESCE, without RPR:

"id" here is a reference to the join's own merged column (a join alias
Var). Unlike LEFT/RIGHT/INNER, a FULL JOIN has no side guaranteed to
survive (either can be NULL), so what that alias Var actually expands to
is the real two-column composite CoalesceExpr(g1.id, g2.id). So whether
GROUP BY spells it by the join's own name or by that expanded COALESCE,
core treats them as the same value:

SELECT id + 1 AS b, count(*) FROM g1 FULL JOIN g2 USING (id)
GROUP BY id + 1 ORDER BY 1; -- 9 rows
SELECT id + 1 AS b, count(*) FROM g1 FULL JOIN g2 USING (id)
GROUP BY COALESCE(g1.id, g2.id) + 1 ORDER BY 1; -- 9 rows

The second query passes because core's grouping-validity check flattens
join alias Vars before comparing (flatten_join_alias_for_parser() when
hasJoinRTEs, then substitute_grouped_columns(), parse_agg.c). Once
SELECT's "id" is flattened, it has the same tree shape as GROUP BY's own
"COALESCE(g1.id, g2.id)".

(SELECT is bare "id" in both queries above -- this equivalence is about
GROUP BY's spelling, not something SELECT itself needs to adopt.)

Case 1 -- with DEFINE in the picture, that equivalence breaks:

First, spelling GROUP BY the same way as DEFINE ("id + 1") runs fine and
returns 9 rows:

SELECT id + 1 AS b, count(*) OVER w AS cnt
FROM g1 FULL JOIN g2 USING (id)
GROUP BY id + 1
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A) DEFINE A AS (id + 1) > 0)
ORDER BY 1;

b | cnt
----+-----
2 | 1
3 | 1
4 | 1
5 | 1
6 | 1
7 | 1
8 | 1
9 | 1
10 | 1
(9 rows)

Switching only GROUP BY to the expanded COALESCE spelling -- leaving
DEFINE's spelling as is -- reproduces it:

SELECT id + 1 AS b, count(*) OVER w AS cnt
FROM g1 FULL JOIN g2 USING (id)
GROUP BY COALESCE(g1.id, g2.id) + 1
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A) DEFINE A AS (id + 1) > 0);

This query is currently rejected:

ERROR: column "g1.id" must appear in the GROUP BY clause or be used in
an aggregate function

with no error position -- the reference at fault isn't anything the user
wrote, it's the resjunk entry that transformDefineClause()/
define_plant_walker() plants for what DEFINE reads.

Root cause: define_plant_walker() (parse_rpr.c) checks whether a DEFINE
subexpression is already computed by GROUP BY via plain equal() against
the collected groupExprs. The CoalesceExpr(g1.id, g2.id) from the
background above sits on the GROUP BY side, while DEFINE reads the same
column through the join's own bare Var; the two trees differ even though
they denote the same value, so the walker doesn't recognize the overlap
and plants a needless resjunk entry -- which then fails PostgreSQL's
normal grouping-validity check.

I wrote and validated a fix: flatten join alias Vars out of both sides
of the comparison (groupExprs, and the node being walked) with the
existing flatten_join_alias_for_parser(), whenever the range table has a
join RTE. Since transformDefineClause() runs before the Query is
assembled, there's no Query to hand that function yet -- a stack-local
stub carrying just p_rtable and p_hasSubLinks is enough (DEFINE can't
contain a SubLink). It passes all five RPR regression suites and
pgindent leaves it untouched.

Case 2 -- the same scenario, viewed, with that fix applied:

CREATE VIEW rpr_v2 AS
SELECT COALESCE(rpr_grp.id, rpr_sort.id) + 1 AS idp1, count(*) OVER w AS cnt
FROM rpr_grp FULL JOIN rpr_sort USING (id)
GROUP BY COALESCE(rpr_grp.id, rpr_sort.id) + 1
WINDOW w AS (
ORDER BY COALESCE(rpr_grp.id, rpr_sort.id) + 1
ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A+)
DEFINE A AS id + 1 > 0);

CREATE VIEW itself now succeeds (thanks to the fix). But pg_get_viewdef()
prints the DEFINE clause as:

DEFINE a AS (COALESCE(id, id) + 1) > 0

DEFINE forbids qualified references, so whatever prints the merged
column's two sides (l.id and r.id) has to print them the same way --
here, both as bare "id". Re-parsing that text doesn't recover the
original one-layer CoalesceExpr(l.id, r.id): it re-resolves the bare
"id" as the join's merged column again, producing a doubly-wrapped
COALESCE(id, id) around the join's own COALESCE(l.id, r.id). Re-parsing
fails on the **same** error as case 1 -- just on a tree that's now one
layer deeper.

DEFINE's ban on qualified references is presumably there because a
pattern-matching row doesn't have separate "sides" to qualify against,
and a FULL JOIN's USING column is a genuine exception: it's built from
two distinct columns. Still, the grammar doesn't need to change.

A qualified reference like "g1.id" can never appear inside DEFINE as the
user wrote it -- parse analysis rejects it outright ("range variable
qualified expression ... is not allowed in DEFINE clause"). So if
DEFINE's stored expression ever contains a qualified CoalesceExpr, it can
only have arrived via the GROUP BY substitution described above. And for
a FULL JOIN's USING column, that expanded form is exactly the join's own
merged-column definition (RangeTblEntry.joinaliasvars) -- the very
expression the join itself already records as "how this column is
computed."

So at deparse time, instead of printing what flatten_group_exprs()
expanded to as-is, I match it against every join RTE's joinaliasvars in
the range table, and when it matches exactly, fold it back into a plain
Var referencing that join column (collapse_define_join_vars_mutator(),
ruleutils.c). The fix stays inside deparse -- the grammar and DEFINE's
execution path are untouched.

pg_get_viewdef() now prints:

DEFINE a AS (id + 1) > 0

with no COALESCE, and recreating the view from that text produces an
identical pg_get_viewdef() output -- it round-trips cleanly. I've
applied case 1's fix alongside this, and all five RPR regression suites
(rpr, rpr_base, rpr_explain, rpr_integration, rpr_nfa) pass. I've added
both cases (query succeeds, view round-trips) as regression tests in
rpr_base.

Patch attached (includes the rpr_base regression tests).

I'm not sure whether applying this patch is the right call, though, versus
just fixing the error message instead. Even before RPR, I'm not confident
whether core flattening join alias Vars to recognize a mismatched GROUP BY
spelling is an intended guarantee, or just happens to work that way. If
it's the latter, DEFINE's current rejection might be the more honest
behavior, and the right fix might be turning today's positionless error
into a positioned one instead of this patch.

I'd appreciate your take.

Best regards,
Henson