public inbox for [email protected]
help / color / mirror / Atom feedFrom: Richard Guo <[email protected]>
To: Tom Lane <[email protected]>
Cc: Swirl Smog Dowry <[email protected]>
Cc: [email protected]
Subject: Re: pg_get_viewdef() produces non-round-trippable SQL for views with USING join on mismatched integer types
Date: Fri, 27 Feb 2026 12:23:14 +0900
Message-ID: <CAMbWs4_FLicDsNpotM0uEFkeveJ+gULe4VP0PyJ=FVsD9iYCGA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+-gibjCg_vjcq3hWTM0sLs3_TUZ6Q9rkv8+pe2yJrdh4o4uoQ@mail.gmail.com>
<[email protected]>
<[email protected]>
On Fri, Feb 27, 2026 at 3:27 AM Tom Lane <[email protected]> wrote:
> The problem is obvious after looking at parseCheckAggregates: the
> RTE_GROUP RTE is manufactured using the groupClauses list after we
> have flattened that, which we are only doing for comparison purposes;
> it shouldn't affect what goes into the parse tree. I experimented
> with just changing the order of operations, and that seems to fix it.
Right. We should keep the unmodified GROUP BY expressions in the
parse tree, and then rely on the planner to flatten the join alias
vars within them.
+1 to the fix.
> The lack of any effect on check-world shows we need more test cases
> here ...
How about this new test case in the attached (parse_agg.c untouched)?
- Richard
Attachments:
[application/octet-stream] wip-preserve-join-aliases-in-RTE_GROUP.patch (3.4K, 2-wip-preserve-join-aliases-in-RTE_GROUP.patch)
download | inline diff:
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 25ee0f87d93..d0187ea84a0 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -1213,8 +1213,8 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
}
/*
- * Build a list of the acceptable GROUP BY expressions for use by
- * substitute_grouped_columns().
+ * Build a list of the acceptable GROUP BY expressions to save in the
+ * RTE_GROUP RTE, and for use by substitute_grouped_columns().
*
* We get the TLE, not just the expr, because GROUPING wants to know the
* sortgroupref.
@@ -1231,6 +1231,23 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
groupClauses = lappend(groupClauses, expr);
}
+ /*
+ * If there are any acceptable GROUP BY expressions, build an RTE and
+ * nsitem for the result of the grouping step. (It's important to do this
+ * before flattening join alias vars in groupClauses, because the RTE
+ * should preserve any alias vars that were in the input.)
+ */
+ if (groupClauses)
+ {
+ pstate->p_grouping_nsitem =
+ addRangeTableEntryForGroup(pstate, groupClauses);
+
+ /* Set qry->rtable again in case it was previously NIL */
+ qry->rtable = pstate->p_rtable;
+ /* Mark the Query as having RTE_GROUP RTE */
+ qry->hasGroupRTE = true;
+ }
+
/*
* If there are join alias vars involved, we have to flatten them to the
* underlying vars, so that aliased and unaliased vars will be correctly
@@ -1266,21 +1283,6 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
}
}
- /*
- * If there are any acceptable GROUP BY expressions, build an RTE and
- * nsitem for the result of the grouping step.
- */
- if (groupClauses)
- {
- pstate->p_grouping_nsitem =
- addRangeTableEntryForGroup(pstate, groupClauses);
-
- /* Set qry->rtable again in case it was previously NIL */
- qry->rtable = pstate->p_rtable;
- /* Mark the Query as having RTE_GROUP RTE */
- qry->hasGroupRTE = true;
- }
-
/*
* Replace grouped variables in the targetlist and HAVING clause with Vars
* that reference the RTE_GROUP RTE. Emit an error message if we find any
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index cae8e7bca31..7b2b462804b 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1794,6 +1794,18 @@ group by f2;
----+-------
(0 rows)
+-- check that we preserve join alias in GROUP BY expressions
+create temp view v1 as select f1::int from t1 left join t2 using (f1) group by f1;
+select pg_get_viewdef('v1'::regclass);
+ pg_get_viewdef
+-------------------------------
+ SELECT (f1)::integer AS f1 +
+ FROM (t1 +
+ LEFT JOIN t2 USING (f1))+
+ GROUP BY f1;
+(1 row)
+
+drop view v1;
drop table t1, t2;
--
-- Test planner's selection of pathkeys for ORDER BY aggregates
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 850f5a5787f..266ef40e420 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -649,6 +649,11 @@ select f2, count(*) from
t1 x(x0,x1) left join (t1 left join t2 using(f2)) on (x0 = 0)
group by f2;
+-- check that we preserve join alias in GROUP BY expressions
+create temp view v1 as select f1::int from t1 left join t2 using (f1) group by f1;
+select pg_get_viewdef('v1'::regclass);
+
+drop view v1;
drop table t1, t2;
--
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_get_viewdef() produces non-round-trippable SQL for views with USING join on mismatched integer types
In-Reply-To: <CAMbWs4_FLicDsNpotM0uEFkeveJ+gULe4VP0PyJ=FVsD9iYCGA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox