v3-0005-Teach-group_keys_reorder_by_pathkeys-about-redund.patch

From d76a85b614f5d27c603af812799104a006076cbe Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Tue, 21 May 2024 03:13:11 +0300
Subject: [PATCH v3 5/5] 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.

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

diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 4287811abe4..6ae595781d8 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,
@@ -545,6 +557,7 @@ get_useful_group_keys_orderings(PlannerInfo *root, Path *path)
 		}
 	}
 #endif
+
 	return infos;
 }
 
-- 
2.39.3 (Apple Git-145)