Re: "could not find pathkey item to sort" for TPC-DS queries 94-96 - Mailing list pgsql-hackers

ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>> +    /* We ignore binary-compatible relabeling on both ends */
>> +    while (expr && IsA(expr, RelabelType))
>> +        expr = ((RelabelType *) expr)->arg;
>
> There are 10 instances of this exact loop scattered around the codebase.
> Is it worth it turning it into a static inline function?

Something like the attached, maybe?

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

From b481a41e494f169765ee204aa17ba60c16950455 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 20 Apr 2021 12:08:46 +0100
Subject: [PATCH] Create a function for stripping RelabelType nodes off an
 expression

The exact same while loop was repeated 10 times across the codebase,
which makes it smell like time to refactor.
---
 contrib/postgres_fdw/postgres_fdw.c     |  7 ++-----
 src/backend/nodes/nodeFuncs.c           |  3 +--
 src/backend/optimizer/path/equivclass.c |  4 +---
 src/backend/optimizer/plan/createplan.c |  8 ++------
 src/backend/optimizer/plan/initsplan.c  | 12 +++++-------
 src/backend/optimizer/util/tlist.c      |  8 ++------
 src/include/nodes/nodeFuncs.h           |  9 +++++++++
 7 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index c590f374c6..5c45b2b087 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -7203,8 +7203,7 @@ find_em_expr_for_input_target(PlannerInfo *root,
         }
 
         /* We ignore binary-compatible relabeling on both ends */
-        while (expr && IsA(expr, RelabelType))
-            expr = ((RelabelType *) expr)->arg;
+        expr = (Expr *) strip_relabeltype(expr);
 
         /* Locate an EquivalenceClass member matching this expr, if any */
         foreach(lc2, ec->ec_members)
@@ -7221,9 +7220,7 @@ find_em_expr_for_input_target(PlannerInfo *root,
                 continue;
 
             /* Match if same expression (after stripping relabel) */
-            em_expr = em->em_expr;
-            while (em_expr && IsA(em_expr, RelabelType))
-                em_expr = ((RelabelType *) em_expr)->arg;
+            em_expr = (Expr *) strip_relabeltype(em->em_expr);
 
             if (equal(em_expr, expr))
                 return em->em_expr;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index ff3dcc7b18..9918a4c12e 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -587,8 +587,7 @@ applyRelabelType(Node *arg, Oid rtype, int32 rtypmod, Oid rcollid,
      * all but the top one, and must do so to ensure that semantically
      * equivalent expressions are equal().
      */
-    while (arg && IsA(arg, RelabelType))
-        arg = (Node *) ((RelabelType *) arg)->arg;
+    arg = strip_relabeltype(arg);
 
     if (arg && IsA(arg, Const))
     {
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 0188c1e9a1..2f78998a82 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -2307,9 +2307,7 @@ match_eclasses_to_foreign_key_col(PlannerInfo *root,
                 continue;        /* ignore children here */
 
             /* EM must be a Var, possibly with RelabelType */
-            var = (Var *) em->em_expr;
-            while (var && IsA(var, RelabelType))
-                var = (Var *) ((RelabelType *) var)->arg;
+            var = (Var *) strip_relabeltype(em->em_expr);
             if (!(var && IsA(var, Var)))
                 continue;
 
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 22f10fa339..b13aa65244 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -6257,9 +6257,7 @@ find_ec_member_for_tle(EquivalenceClass *ec,
     ListCell   *lc;
 
     /* We ignore binary-compatible relabeling on both ends */
-    tlexpr = tle->expr;
-    while (tlexpr && IsA(tlexpr, RelabelType))
-        tlexpr = ((RelabelType *) tlexpr)->arg;
+    tlexpr = (Expr *) strip_relabeltype(tle->expr);
 
     foreach(lc, ec->ec_members)
     {
@@ -6281,9 +6279,7 @@ find_ec_member_for_tle(EquivalenceClass *ec,
             continue;
 
         /* Match if same expression (after stripping relabel) */
-        emexpr = em->em_expr;
-        while (emexpr && IsA(emexpr, RelabelType))
-            emexpr = ((RelabelType *) emexpr)->arg;
+        emexpr = (Expr *) strip_relabeltype(em->em_expr);
 
         if (equal(emexpr, tlexpr))
             return em;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 3ac853d9ef..84805906e4 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2566,16 +2566,14 @@ match_foreign_keys_to_quals(PlannerInfo *root)
                 if (!IsA(clause, OpExpr) ||
                     list_length(clause->args) != 2)
                     continue;
-                leftvar = (Var *) get_leftop((Expr *) clause);
-                rightvar = (Var *) get_rightop((Expr *) clause);
 
-                /* Operands must be Vars, possibly with RelabelType */
-                while (leftvar && IsA(leftvar, RelabelType))
-                    leftvar = (Var *) ((RelabelType *) leftvar)->arg;
+                /* Type relabeling doesn't matter */
+                leftvar = (Var *) strip_relabeltype(get_leftop((Expr *) clause));
+                rightvar = (Var *) strip_relabeltype(get_rightop((Expr *) clause));
+
+                /* Operands must be Vars */
                 if (!(leftvar && IsA(leftvar, Var)))
                     continue;
-                while (rightvar && IsA(rightvar, RelabelType))
-                    rightvar = (Var *) ((RelabelType *) rightvar)->arg;
                 if (!(rightvar && IsA(rightvar, Var)))
                     continue;
 
diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 8a26288070..6ea3e1da03 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -90,16 +90,12 @@ tlist_member_ignore_relabel(Expr *node, List *targetlist)
 {
     ListCell   *temp;
 
-    while (node && IsA(node, RelabelType))
-        node = ((RelabelType *) node)->arg;
+    node = (Expr *) strip_relabeltype(node);
 
     foreach(temp, targetlist)
     {
         TargetEntry *tlentry = (TargetEntry *) lfirst(temp);
-        Expr       *tlexpr = tlentry->expr;
-
-        while (tlexpr && IsA(tlexpr, RelabelType))
-            tlexpr = ((RelabelType *) tlexpr)->arg;
+        Expr       *tlexpr = (Expr *) strip_relabeltype(tlentry->expr);
 
         if (equal(node, tlexpr))
             return tlentry;
diff --git a/src/include/nodes/nodeFuncs.h b/src/include/nodes/nodeFuncs.h
index 03a346c01d..b11649d283 100644
--- a/src/include/nodes/nodeFuncs.h
+++ b/src/include/nodes/nodeFuncs.h
@@ -126,6 +126,15 @@ get_notclausearg(const void *notclause)
     return (Expr *) linitial(((const BoolExpr *) notclause)->args);
 }
 
+/* Strip off any RelabelType nodes */
+static inline Node *
+strip_relabeltype(const void *node)
+{
+    while (node && IsA(node, RelabelType))
+        node = ((RelabelType *) node)->arg;
+    return (Node *) node;
+}
+
 extern bool check_functions_in_node(Node *node, check_function_callback checker,
                                     void *context);
 
-- 
2.29.2