Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mzRuo-0006yU-9t for pgsql-hackers@arkaria.postgresql.org; Mon, 20 Dec 2021 23:17:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mzRum-0002Cs-MV for pgsql-hackers@arkaria.postgresql.org; Mon, 20 Dec 2021 23:17:56 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mzRum-0002Cj-DH for pgsql-hackers@lists.postgresql.org; Mon, 20 Dec 2021 23:17:56 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mzRuh-0007J1-H5 for pgsql-hackers@lists.postgresql.org; Mon, 20 Dec 2021 23:17:55 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 1BKNHnnM2674605; Mon, 20 Dec 2021 18:17:49 -0500 From: Tom Lane To: Robert Haas cc: Justin Pryzby , PostgreSQL Hackers Subject: Re: sqlsmith: ERROR: XX000: bogus varno: 2 In-reply-to: <2657967.1640035211@sss.pgh.pa.us> References: <20211219205422.GT17618@telsasoft.com> <2209221.1639948636@sss.pgh.pa.us> <2445883.1640017555@sss.pgh.pa.us> <2524668.1640024031@sss.pgh.pa.us> <2598851.1640028995@sss.pgh.pa.us> <2657967.1640035211@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Mon, 20 Dec 2021 16:20:11 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2674579.1640042247.0@sss.pgh.pa.us> Date: Mon, 20 Dec 2021 18:17:49 -0500 Message-ID: <2674604.1640042269@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2674579.1640042247.1@sss.pgh.pa.us> Here's a less hasty version of the patch. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="check-for-bogus-vars-2.patch"; charset="us-ascii" Content-ID: <2674579.1640042247.2@sss.pgh.pa.us> Content-Description: check-for-bogus-vars-2.patch Content-Transfer-Encoding: quoted-printable 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 =3D (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 =3D (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 =3D (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 =3D (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 relid= s); * 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 f= ind * bare SubLinks, and if so it needs to recurse into them to look for upl= evel * references to the desired rtable level! But when we find a completed @@ -168,9 +171,13 @@ pull_varnos_walker(Node *node, pull_varnos_context *c= ontext) /* * 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 =3D=3D context->sublevels_up) + if (phv->phlevelsup =3D=3D context->sublevels_up && + context->root !=3D 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/rul= eutils.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_arr= ay, 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 pretty= Flags) { 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 cha= r *relname, int prettyFlags) = pfree(exprstr); = + /* + * Throw error if the input is a querytree rather than an expression tre= e. + * 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, o= r + * even nested Lists, so drill down to a non-List node before checking. + */ + tst =3D node; + while (tst && IsA(tst, List)) + tst =3D 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 =3D 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 =3D deparse_context_for(relname, relid); ------- =_aaaaaaaaaa0--