Redundant parameter in the get_useful_pathkeys_for_relation
Hi,
Working on an unrelated project, I found that the
get_useful_pathkeys_for_relation routine has redundant parameter
'require_parallel_safe'. This routine is static and is called only once
with the 'true' value. Hence, it is evident that this parameter is a
redundant one. The thread [1]/messages/by-id/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com seems not to explain why it is necessary.
Although this function may be reused someday, I propose to remove this
parameter or, at least, clarify why it is required here.
I would also remove the same parameter from the interface of routine
relation_can_be_sorted_early, but it is exported, and someone may
already use it in a loadable module.
See the patch attached.
[1]: /messages/by-id/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
/messages/by-id/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
--
regards, Andrei Lepikhov
Attachments:
0001-Remove-redundant-parameter-from-get_useful_pathkeys_.patchtext/plain; charset=UTF-8; name=0001-Remove-redundant-parameter-from-get_useful_pathkeys_.patchDownload
From 8f437f5edf1c832cc2efd615952251a8e50af2e7 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepihov@gmail.com>
Date: Tue, 26 Aug 2025 15:51:59 +0200
Subject: [PATCH] Remove redundant parameter from
get_useful_pathkeys_for_relation
---
src/backend/optimizer/path/allpaths.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 6cc6966b060..f4802797f7f 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -3149,8 +3149,8 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
* This allows us to do incremental sort on top of an index scan under a gather
* merge node, i.e. parallelized.
*
- * If the require_parallel_safe is true, we also require the expressions to
- * be parallel safe (which allows pushing the sort below Gather Merge).
+ * Require the expressions to be parallel safe (which allows pushing the sort
+ * below Gather Merge).
*
* XXX At the moment this can only ever return a list with a single element,
* because it looks at query_pathkeys only. So we might return the pathkeys
@@ -3159,8 +3159,7 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
* merge joins.
*/
static List *
-get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel,
- bool require_parallel_safe)
+get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
{
List *useful_pathkeys_list = NIL;
@@ -3190,10 +3189,9 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel,
* that meets this requirement, as we may be able to do an
* incremental sort.
*
- * If requested, ensure the sort expression is parallel-safe too.
+ * Ensure the sort expression is parallel-safe.
*/
- if (!relation_can_be_sorted_early(root, rel, pathkey_ec,
- require_parallel_safe))
+ if (!relation_can_be_sorted_early(root, rel, pathkey_ec, true))
break;
npathkeys++;
@@ -3247,7 +3245,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
generate_gather_paths(root, rel, override_rows);
/* consider incremental sort for interesting orderings */
- useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel, true);
+ useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel);
/* used for explicit (full) sort paths */
cheapest_partial_path = linitial(rel->partial_pathlist);
--
2.51.0