public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[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: Thu, 26 Feb 2026 13:27:10 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+-gibjCg_vjcq3hWTM0sLs3_TUZ6Q9rkv8+pe2yJrdh4o4uoQ@mail.gmail.com>
	<[email protected]>

I wrote:
> The first groupexpr is the same as the joinaliasvars entry for that
> column in the JOIN RTE.  This surprises me: I'd expect to see a
> reference to the join output column there, ie Var 3/1, because I'm
> pretty sure that's what parsing of "GROUP BY year" would have produced
> initially.  If it were like that, I think ruleutils would produce the
> desired output.  So I'd tentatively classify this as "join alias Vars
> are being flattened too soon".  Richard, any thoughts?

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.
The lack of any effect on check-world shows we need more test cases
here ...

			regards, tom lane



Attachments:

  [text/x-diff] wip-preserve-join-aliases-in-RTE_GROUP.patch (2.1K, 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

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: <[email protected]>

* 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