From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: sqlsmith: ERROR: XX000: bogus varno: 2
Date: Mon, 20 Dec 2021 11:25:55 -0500
Message-ID: <2445883.1640017555@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+TgmobHkyGKM_DOgzGbh7BiWXdBGLMTnmB2S_FyyNHgqTxqBg@mail.gmail.com>
References: <20211219205422.GT17618@telsasoft.com>
<2209221.1639948636@sss.pgh.pa.us>
<CA+TgmobHkyGKM_DOgzGbh7BiWXdBGLMTnmB2S_FyyNHgqTxqBg@mail.gmail.com>
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
Attachments:check-for-bogus-vars.patch (text/x-diff, 2.9K) downloaddiff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.cindex 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.cindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, pryzby@telsasoft.com, pgsql-hackers@lists.postgresql.org
Subject: Re: sqlsmith: ERROR: XX000: bogus varno: 2
In-Reply-To: <2445883.1640017555@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox