From b5ea90e50faf8a7a8fc2329a35da5f66fbe3d35b Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Tue, 21 May 2024 03:13:11 +0300
Subject: [PATCH v2 2/4] Teach group_keys_reorder_by_pathkeys() about redundant
 SortGroupClause's

We don't have strict guarantees that there shouldn't be redundant
SortGroupClause's.  Although there is no confirmed way to generate
duplicate SortGroupClause's, this commit teaches
group_keys_reorder_by_pathkeys() to pull all the SortGroupClause matching to
pathkey at once.

This commit also introduces checking invariants of generated orderings
in get_useful_group_keys_orderings for assert-enabled builds.

Discussion: https://postgr.es/m/a663f0f6-cbf6-49aa-af2e-234dc6768a07%40postgrespro.ru
Author: Andrei Lepikhov
---
 src/backend/optimizer/path/pathkeys.c | 69 +++++++++++++++++++++------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 7858a56fae0..e8ff2297697 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -401,7 +401,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
 	foreach(lc, pathkeys)
 	{
 		PathKey    *pathkey = (PathKey *) lfirst(lc);
-		SortGroupClause *sgc;
+		bool		found = false;
 
 		/*
 		 * Pathkeys are built in a way that allows simply comparing pointers.
@@ -416,28 +416,40 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
 		/*
 		 * Since 1349d27 pathkey coming from underlying node can be in the
 		 * root->group_pathkeys but not in the processed_groupClause. So, we
-		 * should be careful here.
+		 * should be careful here.  Also, there could be redundant
+		 * SortGroupClause's.  So, we need to pull of all the matching
+		 * SortGroupClause's, but ensure there is at least one.
 		 */
-		sgc = get_sortgroupref_clause_noerr(pathkey->pk_eclass->ec_sortref,
-											*group_clauses);
-		if (!sgc)
-			/* The grouping clause does not cover this pathkey */
-			break;
+		foreach_ptr(SortGroupClause, sgc, *group_clauses)
+		{
+			if (sgc->tleSortGroupRef == pathkey->pk_eclass->ec_sortref)
+			{
+				/*
+				 * Sort group clause should have an ordering operator as long
+				 * as there is an associated pathkey.
+				 */
+				Assert(OidIsValid(sgc->sortop));
 
-		/*
-		 * Sort group clause should have an ordering operator as long as there
-		 * is an associated pathkey.
-		 */
-		Assert(OidIsValid(sgc->sortop));
+				new_group_clauses = lappend(new_group_clauses, sgc);
+				found = true;
+			}
+		}
+
+		/* Check if the grouping clause does not cover this pathkey */
+		if (!found)
+			break;
 
 		new_group_pathkeys = lappend(new_group_pathkeys, pathkey);
-		new_group_clauses = lappend(new_group_clauses, sgc);
+
 	}
 
 	/* remember the number of pathkeys with a matching GROUP BY key */
 	n = list_length(new_group_pathkeys);
 
-	/* append the remaining group pathkeys (will be treated as not sorted) */
+	/*
+	 * Append the remaining group pathkeys (will be treated as not sorted) and
+	 * grouping clauses.
+	 */
 	*group_pathkeys = list_concat_unique_ptr(new_group_pathkeys,
 											 *group_pathkeys);
 	*group_clauses = list_concat_unique_ptr(new_group_clauses,
@@ -561,6 +573,35 @@ get_useful_group_keys_orderings(PlannerInfo *root, Path *path)
 		}
 	}
 
+#ifdef USE_ASSERT_CHECKING
+	{
+		PathKeyInfo *pinfo = linitial_node(PathKeyInfo, infos);
+		ListCell   *lc;
+
+		/* Test consistency of info structures */
+		for_each_from(lc, infos, 1)
+		{
+			ListCell   *lc1,
+					   *lc2;
+
+			info = lfirst_node(PathKeyInfo, lc);
+
+			Assert(list_length(info->clauses) == list_length(pinfo->clauses));
+			Assert(list_length(info->pathkeys) == list_length(pinfo->pathkeys));
+			Assert(list_difference(info->clauses, pinfo->clauses) == NIL);
+			Assert(list_difference_ptr(info->pathkeys, pinfo->pathkeys) == NIL);
+
+			forboth(lc1, info->clauses, lc2, info->pathkeys)
+			{
+				SortGroupClause *sgc = lfirst_node(SortGroupClause, lc1);
+				PathKey    *pk = lfirst_node(PathKey, lc2);
+
+				if (pk->pk_eclass->ec_sortref != sgc->tleSortGroupRef)
+					elog(ERROR, "Order of group-by clauses doesn't correspond incoming sort order");
+			}
+		}
+	}
+#endif
 	return infos;
 }
 
-- 
2.39.3 (Apple Git-145)