From 8011824b1e82c14c6f22ced9b12aa47ff7fb5ec6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Villemain?= <Cedric.Villemain@Data-Bene.io>
Date: Fri, 13 Sep 2024 23:17:57 +0200
Subject: [PATCH] Fix #7684 : order "planner" tree

Similar to other bugs affected by citus usage of ruletuils: the provided
tree as input is distinct from the tree expected in ruleutils in
PostgreSQL.
As a consequence, it is required in some places to re-order the tree
structure to make it compliant or "back" to the parser tree (before it's
rewritten and reordered by PostgreSQL to optimize execution). Or to
lookup the target list and ensure it's the one "we expect".
Fox this bug, the `get_rule_sortgroupclause()` is used to check if
target list entry `resname` is defined, and use it directly if it
exists.

No benchmark where run, it's not expected to impact a lot.
---
 .../distributed/deparser/ruleutils_14.c        | 18 +++++++++++++++---
 .../distributed/deparser/ruleutils_15.c        | 18 +++++++++++++++---
 .../distributed/deparser/ruleutils_16.c        | 18 +++++++++++++++---
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/backend/distributed/deparser/ruleutils_14.c b/src/backend/distributed/deparser/ruleutils_14.c
index 88948cff5..5410d10cb 100644
--- a/src/backend/distributed/deparser/ruleutils_14.c
+++ b/src/backend/distributed/deparser/ruleutils_14.c
@@ -2462,9 +2462,21 @@ get_basic_select_query(Query *query, deparse_context *context,
 			{
 				SortGroupClause *srt = (SortGroupClause *) lfirst(l);
 
-				appendStringInfoString(buf, sep);
-				get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
-										 false, context);
+				/* Patch: Ensure the correct target list is used without alias swapping */
+				TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);
+
+				/* Ensure the attribute names are preserved */
+				if (tle && tle->resname != NULL)
+				{
+					appendStringInfoString(buf, sep);
+					appendStringInfoString(buf, quote_identifier(tle->resname));
+				}
+				else
+				{
+					/* Fallback to the default behavior */
+					get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
+				}
+
 				sep = ", ";
 			}
 			appendStringInfoChar(buf, ')');
diff --git a/src/backend/distributed/deparser/ruleutils_15.c b/src/backend/distributed/deparser/ruleutils_15.c
index 018468d0b..0eb8f8a49 100644
--- a/src/backend/distributed/deparser/ruleutils_15.c
+++ b/src/backend/distributed/deparser/ruleutils_15.c
@@ -2551,9 +2551,21 @@ get_basic_select_query(Query *query, deparse_context *context,
 			{
 				SortGroupClause *srt = (SortGroupClause *) lfirst(l);
 
-				appendStringInfoString(buf, sep);
-				get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
-										 false, context);
+				/* Patch: Ensure the correct target list is used without alias swapping */
+				TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);
+
+				/* Ensure the attribute names are preserved */
+				if (tle && tle->resname != NULL)
+				{
+					appendStringInfoString(buf, sep);
+					appendStringInfoString(buf, quote_identifier(tle->resname));
+				}
+				else
+				{
+					/* Fallback to the default behavior */
+					get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
+				}
+
 				sep = ", ";
 			}
 			appendStringInfoChar(buf, ')');
diff --git a/src/backend/distributed/deparser/ruleutils_16.c b/src/backend/distributed/deparser/ruleutils_16.c
index 7f2a41d75..b31468bed 100644
--- a/src/backend/distributed/deparser/ruleutils_16.c
+++ b/src/backend/distributed/deparser/ruleutils_16.c
@@ -2565,9 +2565,21 @@ get_basic_select_query(Query *query, deparse_context *context,
 			{
 				SortGroupClause *srt = (SortGroupClause *) lfirst(l);
 
-				appendStringInfoString(buf, sep);
-				get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
-										 false, context);
+				/* Patch: Ensure the correct target list is used without alias swapping */
+				TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);
+
+				/* Ensure the attribute names are preserved */
+				if (tle && tle->resname != NULL)
+				{
+					appendStringInfoString(buf, sep);
+					appendStringInfoString(buf, quote_identifier(tle->resname));
+				}
+				else
+				{
+					/* Fallback to the default behavior */
+					get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
+				}
+
 				sep = ", ";
 			}
 			appendStringInfoChar(buf, ')');