Suppress unnecessary RelabelType nodes in yet more cases.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 19 Aug 2020 18:07:49 +0000 (14:07 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 19 Aug 2020 18:07:49 +0000 (14:07 -0400)
Commit a477bfc1d fixed eval_const_expressions() to ensure that it
didn't generate unnecessary RelabelType nodes, but I failed to notice
that some other places in the planner had the same issue.  Really
noplace in the planner should be using plain makeRelabelType(), for
fear of generating expressions that should be equal() to semantically
equivalent trees, but aren't.

An example is that because canonicalize_ec_expression() failed
to be careful about this, we could end up with an equivalence class
containing both a plain Const, and a Const-with-RelabelType
representing exactly the same value.  So far as I can tell this led to
no visible misbehavior, but we did waste a bunch of cycles generating
and evaluating "Const = Const-with-RelabelType" to prove such entries
are redundant.

Hence, move the support function added by a477bfc1d to where it can
be more generally useful, and use it in the places where planner code
previously used makeRelabelType.

Back-patch to v12, like the previous patch.  While I have no concrete
evidence of any real misbehavior here, it's certainly possible that
I overlooked a case where equivalent expressions that aren't equal()
could cause a user-visible problem.  In any case carrying extra
RelabelType nodes through planning to execution isn't very desirable.

Discussion: https://postgr.es/m/1311836.1597781384@sss.pgh.pa.us

src/backend/nodes/nodeFuncs.c
src/backend/optimizer/path/equivclass.c
src/backend/optimizer/prep/prepunion.c
src/backend/optimizer/util/clauses.c
src/include/nodes/nodeFuncs.h

index d85ca9f7c50108d290c3477d0181d27ba7112fcd..9ce8f43385ec83227bb633278d38d37ac91b0af8 100644 (file)
@@ -575,27 +575,76 @@ exprIsLengthCoercion(const Node *expr, int32 *coercedTypmod)
    return false;
 }
 
+/*
+ * applyRelabelType
+ *     Add a RelabelType node if needed to make the expression expose
+ *     the specified type, typmod, and collation.
+ *
+ * This is primarily intended to be used during planning.  Therefore, it must
+ * maintain the post-eval_const_expressions invariants that there are not
+ * adjacent RelabelTypes, and that the tree is fully const-folded (hence,
+ * we mustn't return a RelabelType atop a Const).  If we do find a Const,
+ * we'll modify it in-place if "overwrite_ok" is true; that should only be
+ * passed as true if caller knows the Const is newly generated.
+ */
+Node *
+applyRelabelType(Node *arg, Oid rtype, int32 rtypmod, Oid rcollid,
+                CoercionForm rformat, int rlocation, bool overwrite_ok)
+{
+   /*
+    * If we find stacked RelabelTypes (eg, from foo::int::oid) we can discard
+    * 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;
+
+   if (arg && IsA(arg, Const))
+   {
+       /* Modify the Const directly to preserve const-flatness. */
+       Const      *con = (Const *) arg;
+
+       if (!overwrite_ok)
+           con = copyObject(con);
+       con->consttype = rtype;
+       con->consttypmod = rtypmod;
+       con->constcollid = rcollid;
+       /* We keep the Const's original location. */
+       return (Node *) con;
+   }
+   else if (exprType(arg) == rtype &&
+            exprTypmod(arg) == rtypmod &&
+            exprCollation(arg) == rcollid)
+   {
+       /* Sometimes we find a nest of relabels that net out to nothing. */
+       return arg;
+   }
+   else
+   {
+       /* Nope, gotta have a RelabelType. */
+       RelabelType *newrelabel = makeNode(RelabelType);
+
+       newrelabel->arg = (Expr *) arg;
+       newrelabel->resulttype = rtype;
+       newrelabel->resulttypmod = rtypmod;
+       newrelabel->resultcollid = rcollid;
+       newrelabel->relabelformat = rformat;
+       newrelabel->location = rlocation;
+       return (Node *) newrelabel;
+   }
+}
+
 /*
  * relabel_to_typmod
  *     Add a RelabelType node that changes just the typmod of the expression.
  *
- * This is primarily intended to be used during planning.  Therefore, it
- * strips any existing RelabelType nodes to maintain the planner's invariant
- * that there are not adjacent RelabelTypes.
+ * Convenience function for a common usage of applyRelabelType.
  */
 Node *
 relabel_to_typmod(Node *expr, int32 typmod)
 {
-   Oid         type = exprType(expr);
-   Oid         coll = exprCollation(expr);
-
-   /* Strip any existing RelabelType node(s) */
-   while (expr && IsA(expr, RelabelType))
-       expr = (Node *) ((RelabelType *) expr)->arg;
-
-   /* Apply new typmod, preserving the previous exposed type and collation */
-   return (Node *) makeRelabelType((Expr *) expr, type, typmod, coll,
-                                   COERCE_EXPLICIT_CAST);
+   return applyRelabelType(expr, exprType(expr), typmod, exprCollation(expr),
+                           COERCE_EXPLICIT_CAST, -1, false);
 }
 
 /*
index b99cec00cb7a67ce97f0ff0b575bfd9eac7e8b8f..b68a5a0ec7171ecde7355218794c9aa2f85961cb 100644 (file)
@@ -490,10 +490,6 @@ process_equivalence(PlannerInfo *root,
  * work to not label the collation at all in EC members, but this is risky
  * since some parts of the system expect exprCollation() to deliver the
  * right answer for a sort key.)
- *
- * Note this code assumes that the expression has already been through
- * eval_const_expressions, so there are no CollateExprs and no redundant
- * RelabelTypes.
  */
 Expr *
 canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation)
@@ -514,29 +510,24 @@ canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation)
        exprCollation((Node *) expr) != req_collation)
    {
        /*
-        * Strip any existing RelabelType, then add a new one if needed. This
-        * is to preserve the invariant of no redundant RelabelTypes.
-        *
-        * If we have to change the exposed type of the stripped expression,
-        * set typmod to -1 (since the new type may not have the same typmod
-        * interpretation).  If we only have to change collation, preserve the
-        * exposed typmod.
+        * If we have to change the type of the expression, set typmod to -1,
+        * since the new type may not have the same typmod interpretation.
+        * When we only have to change collation, preserve the exposed typmod.
+        */
+       int32       req_typmod;
+
+       if (expr_type != req_type)
+           req_typmod = -1;
+       else
+           req_typmod = exprTypmod((Node *) expr);
+
+       /*
+        * Use applyRelabelType so that we preserve const-flatness.  This is
+        * important since eval_const_expressions has already been applied.
         */
-       while (expr && IsA(expr, RelabelType))
-           expr = (Expr *) ((RelabelType *) expr)->arg;
-
-       if (exprType((Node *) expr) != req_type)
-           expr = (Expr *) makeRelabelType(expr,
-                                           req_type,
-                                           -1,
-                                           req_collation,
-                                           COERCE_IMPLICIT_CAST);
-       else if (exprCollation((Node *) expr) != req_collation)
-           expr = (Expr *) makeRelabelType(expr,
-                                           req_type,
-                                           exprTypmod((Node *) expr),
-                                           req_collation,
-                                           COERCE_IMPLICIT_CAST);
+       expr = (Expr *) applyRelabelType((Node *) expr,
+                                        req_type, req_typmod, req_collation,
+                                        COERCE_IMPLICIT_CAST, -1, false);
    }
 
    return expr;
index 2ebd4ea332071cff60eb34c0b02bd78b1e192baa..745f443e5c2df22927f0928d2c04673d296d5d31 100644 (file)
@@ -1200,13 +1200,9 @@ generate_setop_tlist(List *colTypes, List *colCollations,
         * will reach the executor without any further processing.
         */
        if (exprCollation(expr) != colColl)
-       {
-           expr = (Node *) makeRelabelType((Expr *) expr,
-                                           exprType(expr),
-                                           exprTypmod(expr),
-                                           colColl,
-                                           COERCE_IMPLICIT_CAST);
-       }
+           expr = applyRelabelType(expr,
+                                   exprType(expr), exprTypmod(expr), colColl,
+                                   COERCE_IMPLICIT_CAST, -1, false);
 
        tle = makeTargetEntry((Expr *) expr,
                              (AttrNumber) resno++,
index 7105d0a2db9a50e4e0d41054b8c952081a805e67..750586fceb74628a32771f91907c42d306ad09fa 100644 (file)
@@ -120,9 +120,6 @@ static Node *eval_const_expressions_mutator(Node *node,
 static bool contain_non_const_walker(Node *node, void *context);
 static bool ece_function_is_safe(Oid funcid,
                                 eval_const_expressions_context *context);
-static Node *apply_const_relabel(Node *arg, Oid rtype,
-                                int32 rtypmod, Oid rcollid,
-                                CoercionForm rformat, int rlocation);
 static List *simplify_or_arguments(List *args,
                                   eval_const_expressions_context *context,
                                   bool *haveNull, bool *forceTrue);
@@ -2819,12 +2816,13 @@ eval_const_expressions_mutator(Node *node,
                arg = eval_const_expressions_mutator((Node *) relabel->arg,
                                                     context);
                /* ... and attach a new RelabelType node, if needed */
-               return apply_const_relabel(arg,
-                                          relabel->resulttype,
-                                          relabel->resulttypmod,
-                                          relabel->resultcollid,
-                                          relabel->relabelformat,
-                                          relabel->location);
+               return applyRelabelType(arg,
+                                       relabel->resulttype,
+                                       relabel->resulttypmod,
+                                       relabel->resultcollid,
+                                       relabel->relabelformat,
+                                       relabel->location,
+                                       true);
            }
        case T_CoerceViaIO:
            {
@@ -2971,12 +2969,13 @@ eval_const_expressions_mutator(Node *node,
                arg = eval_const_expressions_mutator((Node *) collate->arg,
                                                     context);
                /* ... and attach a new RelabelType node, if needed */
-               return apply_const_relabel(arg,
-                                          exprType(arg),
-                                          exprTypmod(arg),
-                                          collate->collOid,
-                                          COERCE_IMPLICIT_CAST,
-                                          collate->location);
+               return applyRelabelType(arg,
+                                       exprType(arg),
+                                       exprTypmod(arg),
+                                       collate->collOid,
+                                       COERCE_IMPLICIT_CAST,
+                                       collate->location,
+                                       true);
            }
        case T_CaseExpr:
            {
@@ -3478,12 +3477,13 @@ eval_const_expressions_mutator(Node *node,
                                                    cdomain->resulttype);
 
                    /* Generate RelabelType to substitute for CoerceToDomain */
-                   return apply_const_relabel(arg,
-                                              cdomain->resulttype,
-                                              cdomain->resulttypmod,
-                                              cdomain->resultcollid,
-                                              cdomain->coercionformat,
-                                              cdomain->location);
+                   return applyRelabelType(arg,
+                                           cdomain->resulttype,
+                                           cdomain->resulttypmod,
+                                           cdomain->resultcollid,
+                                           cdomain->coercionformat,
+                                           cdomain->location,
+                                           true);
                }
 
                newcdomain = makeNode(CoerceToDomain);
@@ -3616,58 +3616,6 @@ ece_function_is_safe(Oid funcid, eval_const_expressions_context *context)
    return false;
 }
 
-/*
- * Subroutine for eval_const_expressions: apply RelabelType if needed
- */
-static Node *
-apply_const_relabel(Node *arg, Oid rtype, int32 rtypmod, Oid rcollid,
-                   CoercionForm rformat, int rlocation)
-{
-   /*
-    * If we find stacked RelabelTypes (eg, from foo::int::oid) we can discard
-    * 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;
-
-   if (arg && IsA(arg, Const))
-   {
-       /*
-        * If it's a Const, just modify it in-place; since this is part of
-        * eval_const_expressions, we want to end up with a simple Const not
-        * an expression tree.  We assume the Const is newly generated and
-        * hence safe to modify.
-        */
-       Const      *con = (Const *) arg;
-
-       con->consttype = rtype;
-       con->consttypmod = rtypmod;
-       con->constcollid = rcollid;
-       return (Node *) con;
-   }
-   else if (exprType(arg) == rtype &&
-            exprTypmod(arg) == rtypmod &&
-            exprCollation(arg) == rcollid)
-   {
-       /* Sometimes we find a nest of relabels that net out to nothing. */
-       return arg;
-   }
-   else
-   {
-       /* Nope, gotta have a RelabelType. */
-       RelabelType *newrelabel = makeNode(RelabelType);
-
-       newrelabel->arg = (Expr *) arg;
-       newrelabel->resulttype = rtype;
-       newrelabel->resulttypmod = rtypmod;
-       newrelabel->resultcollid = rcollid;
-       newrelabel->relabelformat = rformat;
-       newrelabel->location = rlocation;
-       return (Node *) newrelabel;
-   }
-}
-
 /*
  * Subroutine for eval_const_expressions: process arguments of an OR clause
  *
index 779906b9b77f9efb78c7ecebefe989a6142d559c..9cc56eecaa3acec3d59c7253bb9f514f6604f72f 100644 (file)
@@ -36,6 +36,9 @@ typedef bool (*check_function_callback) (Oid func_id, void *context);
 extern Oid exprType(const Node *expr);
 extern int32 exprTypmod(const Node *expr);
 extern bool exprIsLengthCoercion(const Node *expr, int32 *coercedTypmod);
+extern Node *applyRelabelType(Node *arg, Oid rtype, int32 rtypmod, Oid rcollid,
+                             CoercionForm rformat, int rlocation,
+                             bool overwrite_ok);
 extern Node *relabel_to_typmod(Node *expr, int32 typmod);
 extern Node *strip_implicit_coercions(Node *node);
 extern bool expression_returns_set(Node *clause);