Robert Haas <robertmh...@gmail.com> writes:
> On Sun, Dec 19, 2021 at 4:17 PM Tom Lane <t...@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);
