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 18:17:49 -0500
Message-ID: <2674604.1640042269@sss.pgh.pa.us> (raw)
In-Reply-To: <2657967.1640035211@sss.pgh.pa.us>
References: <20211219205422.GT17618@telsasoft.com>
<2209221.1639948636@sss.pgh.pa.us>
<CA+TgmobHkyGKM_DOgzGbh7BiWXdBGLMTnmB2S_FyyNHgqTxqBg@mail.gmail.com>
<2445883.1640017555@sss.pgh.pa.us>
<CA+Tgmoa7JFTYxM4AJEk+bM_J4Y+1sxQY+rAN3wE+ZmkbqWkCEA@mail.gmail.com>
<2524668.1640024031@sss.pgh.pa.us>
<CA+Tgmob2wVbfj47aqFxYeTuP_rcO45dip_GU-Hz=BSxu4POgXw@mail.gmail.com>
<2598851.1640028995@sss.pgh.pa.us>
<CA+TgmoYz=M+2Z-heoBzJHAzs6heY=4icZ=-b5z6d2+XUw5CeRA@mail.gmail.com>
<2657967.1640035211@sss.pgh.pa.us>
Here's a less hasty version of the patch.
regards, tom lane
Attachments:check-for-bogus-vars-2.patch (text/x-diff, 5.4K) 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/optimizer/util/var.c b/src/backend/optimizer/util/var.cindex 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.cindex 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);
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: <2674604.1640042269@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