Re: sqlsmith: ERROR: XX000: bogus varno: 2

Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Dec 19, 2021 at 4:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I don't see anything particularly surprising here.  pg_get_expr is only
>> able to cope with expression trees over a single relation, but ON UPDATE
>> rules can refer to both OLD and NEW relations.  Maybe we could make the
>> error message more friendly, but there's not much else to be done,
>> I think.

> +1 for making the error message more friendly.

The problem is that the spot where it's thrown doesn't have a lot of
context.  We can fix that by having pg_get_expr itself check for
out-of-spec Vars before starting the recursion, which adds a bit of
overhead but I don't think we're terribly concerned about that.

I figured this would be just a quick hack in ruleutils.c, but was
dismayed to find the regression tests falling over, because some
bozo neglected to teach nodeFuncs.c about partition expressions.
It might be a good idea to back-patch that part, before we find
some other place that fails.

            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/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8da525c715..5d9596d445 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2622,6 +2625,7 @@ static text *
 pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
 {
     Node       *node;
+    Relids        relids;
     List       *context;
     char       *exprstr;
     char       *str;
@@ -2634,6 +2638,27 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)

     pfree(exprstr);

+    /*
+     * Throw a user-friendly error if the expression contains Vars we won't be
+     * able to deparse.  pull_varnos requires a PlannerInfo, but fortunately
+     * that can be dummy -- it need only contain an empty placeholder_list.
+     */
+    relids = pull_varnos(makeNode(PlannerInfo), 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);