Re: sqlsmith: ERROR: XX000: bogus varno: 2 - Mailing list pgsql-hackers

Here's a less hasty version of the patch.

            regards, tom lane

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e276264882..5d4700430c 100644
--- 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 e57c3aa124..9d5b8b9ab9 100644
--- 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 8da525c715..f73ce7ccb5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2561,6 +2564,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
+ * excluded.  So are expressions in more than one relation, which can
+ * appear in places like pg_rewrite.ev_qual.
  * ----------
  */
 Datum
@@ -2622,6 +2631,8 @@ 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;
@@ -2634,6 +2645,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);