1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-02 09:02:37 +03:00

Make pg_get_expr() more bulletproof.

Since this function is defined to accept pg_node_tree values, it could
get applied to any nodetree that can appear in a cataloged pg_node_tree
column.  Some such cases can't be supported --- for example, its API
doesn't allow providing referents for more than one relation --- but
we should try to throw a user-facing error rather than an internal
error when encountering such a case.

In support of this, extend expression_tree_walker/mutator to be sure
they'll work on any such node tree (which basically means adding
support for relpartbound node types).  That allows us to run pull_varnos
and check for the case of multiple relations before we start processing
the tree.  The alternative of changing the low-level error thrown for an
out-of-range varno isn't appealing, because that could mask actual bugs
in other usages of ruleutils.

Per report from Justin Pryzby.  This is basically cosmetic, so no
back-patch.

Discussion: https://postgr.es/m/20211219205422.GT17618@telsasoft.com
This commit is contained in:
Tom Lane
2022-01-09 12:43:09 -05:00
parent 96a6f11c06
commit 6867f963e3
3 changed files with 94 additions and 3 deletions

View File

@ -2201,6 +2201,26 @@ expression_tree_walker(Node *node,
return true; return true;
} }
break; 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: case T_List:
foreach(temp, (List *) node) foreach(temp, (List *) node)
{ {
@ -3092,6 +3112,28 @@ expression_tree_mutator(Node *node,
return (Node *) newnode; return (Node *) newnode;
} }
break; 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: case T_List:
{ {
/* /*

View File

@ -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. * Create a set of all the distinct varnos present in a parsetree.
* Only varnos that reference level-zero rtable entries are considered. * 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 * 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 * 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 * 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, * If a PlaceHolderVar is not of the target query level, ignore it,
* instead recursing into its expression to see if it contains any * 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 * Ideally, the PHV's contribution to context->varnos is its

View File

@ -2561,6 +2561,12 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
* the one specified by the second parameter. This is sufficient for * the one specified by the second parameter. This is sufficient for
* partial indexes, column default expressions, etc. We also support * partial indexes, column default expressions, etc. We also support
* Var-free expressions, for which the OID can be InvalidOid. * 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
* not supported. Nor are expressions in more than one relation, which
* can appear in places like pg_rewrite.ev_qual.
* ---------- * ----------
*/ */
Datum Datum
@ -2622,11 +2628,13 @@ static text *
pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags) pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
{ {
Node *node; Node *node;
Node *tst;
Relids relids;
List *context; List *context;
char *exprstr; char *exprstr;
char *str; char *str;
/* Convert input TEXT object to C string */ /* Convert input pg_node_tree (really TEXT) object to C string */
exprstr = text_to_cstring(expr); exprstr = text_to_cstring(expr);
/* Convert expression to node tree */ /* Convert expression to node tree */
@ -2634,6 +2642,40 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
pfree(exprstr); 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 */ /* Prepare deparse context if needed */
if (OidIsValid(relid)) if (OidIsValid(relid))
context = deparse_context_for(relname, relid); context = deparse_context_for(relname, relid);