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] 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] https://www.postgresql.org/message-id/flat/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com

--
regards, Andrei Lepikhov
From 8f437f5edf1c832cc2efd615952251a8e50af2e7 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
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

Reply via email to