Make pg_get_expr() more bulletproof.

Since this function is defined to accept pg_node_tree values, it could
get applied to any nodetree that can appear in a cataloged pg_node_tree
column.  Some such cases can't be supported --- for example, its API
doesn't allow providing referents for more than one relation --- but
we should try to throw a user-facing error rather than an internal
error when encountering such a case.

In support of this, extend expression_tree_walker/mutator to be sure
they'll work on any such node tree (which basically means adding
support for relpartbound node types).  That allows us to run pull_varnos
and check for the case of multiple relations before we start processing
the tree.  The alternative of changing the low-level error thrown for an
out-of-range varno isn't appealing, because that could mask actual bugs
in other usages of ruleutils.

Per report from Justin Pryzby.  This is basically cosmetic, so no
back-patch.

Discussion: https://postgr.es/m/20211219205422.GT17618@telsasoft.com

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 7d1a3aff8c1b2f979e532ca25b26ca57b245b42f..acc17da7177b80b6612dd13c6168837a2f4b131d 100644 (file)
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2201,6 +2201,26 @@ expression_tree_walker(Node *node,
                                        return true;
                        }
                        break;
+               case T_PartitionBoundSpec:
+                       {
+                               PartitionBoundSpec *pbs = (PartitionBoundSpec *) node;
+
+                               if (walker(pbs->listdatums, context))
+                                       return true;
+                               if (walker(pbs->lowerdatums, context))
+                                       return true;
+                               if (walker(pbs->upperdatums, context))
+                                       return true;
+                       }
+                       break;
+               case T_PartitionRangeDatum:
+                       {
+                               PartitionRangeDatum *prd = (PartitionRangeDatum *) node;
+
+                               if (walker(prd->value, context))
+                                       return true;
+                       }
+                       break;
                case T_List:
                        foreach(temp, (List *) node)
                        {
@@ -3092,6 +3112,28 @@ expression_tree_mutator(Node *node,
                                return (Node *) newnode;
                        }
                        break;
+               case T_PartitionBoundSpec:
+                       {
+                               PartitionBoundSpec *pbs = (PartitionBoundSpec *) node;
+                               PartitionBoundSpec *newnode;
+
+                               FLATCOPY(newnode, pbs, PartitionBoundSpec);
+                               MUTATE(newnode->listdatums, pbs->listdatums, List *);
+                               MUTATE(newnode->lowerdatums, pbs->lowerdatums, List *);
+                               MUTATE(newnode->upperdatums, pbs->upperdatums, List *);
+                               return (Node *) newnode;
+                       }
+                       break;
+               case T_PartitionRangeDatum:
+                       {
+                               PartitionRangeDatum *prd = (PartitionRangeDatum *) node;
+                               PartitionRangeDatum *newnode;
+
+                               FLATCOPY(newnode, prd, PartitionRangeDatum);
+                               MUTATE(newnode->value, prd->value, Node *);
+                               return (Node *) newnode;
+                       }
+                       break;
                case T_List:
                        {
                                /*

diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index ac048beea09eb67be8e191106140b6a75bd44e94..a0543b7f47b388f254db033fa5131585fec7a0dc 100644 (file)
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -88,6 +88,9 @@ static Relids alias_relid_set(Query *query, Relids relids);
  *             Create a set of all the distinct varnos present in a parsetree.
  *             Only varnos that reference level-zero rtable entries are considered.
  *
+ * "root" can be passed as NULL if it is not necessary to process
+ * PlaceHolderVars.
+ *
  * NOTE: this is used on not-yet-planned expressions.  It may therefore find
  * bare SubLinks, and if so it needs to recurse into them to look for uplevel
  * references to the desired rtable level!     But when we find a completed
@@ -168,9 +171,13 @@ pull_varnos_walker(Node *node, pull_varnos_context *context)
                /*
                 * If a PlaceHolderVar is not of the target query level, ignore it,
                 * instead recursing into its expression to see if it contains any
-                * vars that are of the target level.
+                * vars that are of the target level.  We'll also do that when the
+                * caller doesn't pass a "root" pointer.  (We probably shouldn't see
+                * PlaceHolderVars at all in such cases, but if we do, this is a
+                * reasonable behavior.)
                 */
-               if (phv->phlevelsup == context->sublevels_up)
+               if (phv->phlevelsup == context->sublevels_up &&
+                       context->root != NULL)
                {
                        /*
                         * Ideally, the PHV's contribution to context->varnos is its

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 039f897331c863336fa3f7f397810c5913522920..63b85566822f827adb30941a9372f9de9d32afa5 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2561,6 +2561,12 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
  * the one specified by the second parameter.  This is sufficient for
  * partial indexes, column default expressions, etc.  We also support
  * Var-free expressions, for which the OID can be InvalidOid.
+ *
+ * We expect this function to work, or throw a reasonably clean error,
+ * for any node tree that can appear in a catalog pg_node_tree column.
+ * Query trees, such as those appearing in pg_rewrite.ev_action, are
+ * not supported.  Nor are expressions in more than one relation, which
+ * can appear in places like pg_rewrite.ev_qual.
  * ----------
  */
 Datum
@@ -2622,11 +2628,13 @@ static text *
 pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
 {
        Node       *node;
+       Node       *tst;
+       Relids          relids;
        List       *context;
        char       *exprstr;
        char       *str;
 
-       /* Convert input TEXT object to C string */
+       /* Convert input pg_node_tree (really TEXT) object to C string */
        exprstr = text_to_cstring(expr);
 
        /* Convert expression to node tree */
@@ -2634,6 +2642,40 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
 
        pfree(exprstr);
 
+       /*
+        * Throw error if the input is a querytree rather than an expression tree.
+        * While we could support queries here, there seems no very good reason
+        * to.  In most such catalog columns, we'll see a List of Query nodes, or
+        * even nested Lists, so drill down to a non-List node before checking.
+        */
+       tst = node;
+       while (tst && IsA(tst, List))
+               tst = linitial((List *) tst);
+       if (tst && IsA(tst, Query))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("input is a query, not an expression")));
+
+       /*
+        * Throw error if the expression contains Vars we won't be able to
+        * deparse.
+        */
+       relids = pull_varnos(NULL, node);
+       if (OidIsValid(relid))
+       {
+               if (!bms_is_subset(relids, bms_make_singleton(1)))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("expression contains variables of more than one relation")));
+       }
+       else
+       {
+               if (!bms_is_empty(relids))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("expression contains variables")));
+       }
+
        /* Prepare deparse context if needed */
        if (OidIsValid(relid))
                context = deparse_context_for(relname, relid);