From 092d7ded29f36b0539046b23b81b9f0bf2d637f1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 19 Aug 2012 14:12:16 -0400 Subject: [PATCH] Allow OLD and NEW in multi-row VALUES within rules. Now that we have LATERAL, it's fairly painless to allow this case, which was left as a TODO in the original multi-row VALUES implementation. --- src/backend/optimizer/path/allpaths.c | 4 +- src/backend/parser/analyze.c | 46 +++++++--------- src/backend/parser/parse_relation.c | 3 +- src/backend/utils/adt/ruleutils.c | 57 ++++++++++++++------ src/include/nodes/parsenodes.h | 2 +- src/include/parser/parse_relation.h | 1 + src/test/regress/expected/rules.out | 78 +++++++++++++++++++++++++++ src/test/regress/sql/rules.sql | 20 +++++++ 8 files changed, 164 insertions(+), 47 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index dfb0b38448..23a8afb3d0 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -1221,9 +1221,7 @@ set_values_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte) /* * If it's a LATERAL RTE, it might contain some Vars of the current query - * level, requiring it to be treated as parameterized. (NB: even though - * the parser never marks VALUES RTEs as LATERAL, they could be so marked - * by now, as a result of subquery pullup.) + * level, requiring it to be treated as parameterized. */ if (rte->lateral) { diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 6c3d89a14f..823d3b445a 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -587,6 +587,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) List *exprsLists = NIL; List *collations = NIL; int sublist_length = -1; + bool lateral = false; int i; Assert(selectStmt->intoClause == NULL); @@ -647,25 +648,20 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) collations = lappend_oid(collations, InvalidOid); /* - * Another thing we can't currently support is NEW/OLD references in - * rules --- seems we'd need something like SQL99's LATERAL construct - * to ensure that the values would be available while evaluating the - * VALUES RTE. This is a shame. FIXME + * Ordinarily there can't be any current-level Vars in the expression + * lists, because the namespace was empty ... but if we're inside + * CREATE RULE, then NEW/OLD references might appear. In that case we + * have to mark the VALUES RTE as LATERAL. */ if (list_length(pstate->p_rtable) != 1 && contain_vars_of_level((Node *) exprsLists, 0)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("VALUES must not contain OLD or NEW references"), - errhint("Use SELECT ... UNION ALL ... instead."), - parser_errposition(pstate, - locate_var_of_level((Node *) exprsLists, 0)))); + lateral = true; /* * Generate the VALUES RTE */ rte = addRangeTableEntryForValues(pstate, exprsLists, collations, - NULL, true); + NULL, lateral, true); rtr = makeNode(RangeTblRef); /* assume new rte is at end */ rtr->rtindex = list_length(pstate->p_rtable); @@ -1032,6 +1028,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) List *collations; List **colexprs = NULL; int sublist_length = -1; + bool lateral = false; RangeTblEntry *rte; int rtindex; ListCell *lc; @@ -1176,11 +1173,21 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) list_free(colexprs[i]); } + /* + * Ordinarily there can't be any current-level Vars in the expression + * lists, because the namespace was empty ... but if we're inside CREATE + * RULE, then NEW/OLD references might appear. In that case we have to + * mark the VALUES RTE as LATERAL. + */ + if (pstate->p_rtable != NIL && + contain_vars_of_level((Node *) exprsLists, 0)) + lateral = true; + /* * Generate the VALUES RTE */ rte = addRangeTableEntryForValues(pstate, exprsLists, collations, - NULL, true); + NULL, lateral, true); addRTEtoQuery(pstate, rte, true, true, true); /* assume new rte is at end */ @@ -1214,21 +1221,6 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("SELECT FOR UPDATE/SHARE cannot be applied to VALUES"))); - /* - * Another thing we can't currently support is NEW/OLD references in rules - * --- seems we'd need something like SQL99's LATERAL construct to ensure - * that the values would be available while evaluating the VALUES RTE. - * This is a shame. FIXME - */ - if (list_length(pstate->p_rtable) != 1 && - contain_vars_of_level((Node *) exprsLists, 0)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("VALUES must not contain OLD or NEW references"), - errhint("Use SELECT ... UNION ALL ... instead."), - parser_errposition(pstate, - locate_var_of_level((Node *) exprsLists, 0)))); - qry->rtable = pstate->p_rtable; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 98ebc400d5..d9c73ae5a7 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1313,6 +1313,7 @@ addRangeTableEntryForValues(ParseState *pstate, List *exprs, List *collations, Alias *alias, + bool lateral, bool inFromCl) { RangeTblEntry *rte = makeNode(RangeTblEntry); @@ -1355,7 +1356,7 @@ addRangeTableEntryForValues(ParseState *pstate, * * Subqueries are never checked for access rights. */ - rte->lateral = false; + rte->lateral = lateral; rte->inh = false; /* never true for values RTEs */ rte->inFromCl = inFromCl; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 0103021961..f6f7f85f44 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2919,11 +2919,48 @@ get_select_query_def(Query *query, deparse_context *context, context->windowTList = save_windowtlist; } +/* + * Detect whether query looks like SELECT ... FROM VALUES(); + * if so, return the VALUES RTE. Otherwise return NULL. + */ +static RangeTblEntry * +get_simple_values_rte(Query *query) +{ + RangeTblEntry *result = NULL; + ListCell *lc; + + /* + * We want to return TRUE even if the Query also contains OLD or NEW rule + * RTEs. So the idea is to scan the rtable and see if there is only one + * inFromCl RTE that is a VALUES RTE. We don't look at the targetlist at + * all. This is okay because parser/analyze.c will never generate a + * "bare" VALUES RTE --- they only appear inside auto-generated + * sub-queries with very restricted structure. + */ + foreach(lc, query->rtable) + { + RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc); + + if (rte->rtekind == RTE_VALUES && rte->inFromCl) + { + if (result) + return NULL; /* multiple VALUES (probably not possible) */ + result = rte; + } + else if (rte->rtekind == RTE_RELATION && !rte->inFromCl) + continue; /* ignore rule entries */ + else + return NULL; /* something else -> not simple VALUES */ + } + return result; +} + static void get_basic_select_query(Query *query, deparse_context *context, TupleDesc resultDesc) { StringInfo buf = context->buf; + RangeTblEntry *values_rte; char *sep; ListCell *l; @@ -2936,23 +2973,13 @@ get_basic_select_query(Query *query, deparse_context *context, /* * If the query looks like SELECT * FROM (VALUES ...), then print just the * VALUES part. This reverses what transformValuesClause() did at parse - * time. If the jointree contains just a single VALUES RTE, we assume - * this case applies (without looking at the targetlist...) + * time. */ - if (list_length(query->jointree->fromlist) == 1) + values_rte = get_simple_values_rte(query); + if (values_rte) { - RangeTblRef *rtr = (RangeTblRef *) linitial(query->jointree->fromlist); - - if (IsA(rtr, RangeTblRef)) - { - RangeTblEntry *rte = rt_fetch(rtr->rtindex, query->rtable); - - if (rte->rtekind == RTE_VALUES) - { - get_values_def(rte->values_lists, context); - return; - } - } + get_values_def(values_rte->values_lists, context); + return; } /* diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index f433166cc6..19178b5551 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -758,7 +758,7 @@ typedef struct RangeTblEntry */ Alias *alias; /* user-written alias clause, if any */ Alias *eref; /* expanded reference names */ - bool lateral; /* subquery or function is marked LATERAL? */ + bool lateral; /* subquery, function, or values is LATERAL? */ bool inh; /* inheritance requested? */ bool inFromCl; /* present in FROM clause? */ AclMode requiredPerms; /* bitmask of required access permissions */ diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index ba99fc2d8a..4b3779bdc6 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -67,6 +67,7 @@ extern RangeTblEntry *addRangeTableEntryForValues(ParseState *pstate, List *exprs, List *collations, Alias *alias, + bool lateral, bool inFromCl); extern RangeTblEntry *addRangeTableEntryForJoin(ParseState *pstate, List *colnames, diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index b041550f00..64043e6ead 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1593,3 +1593,81 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier; WHERE sh.slunit = un.un_name; (1 row) +-- +-- check multi-rule VALUES in rules +-- +create table rules_src(f1 int, f2 int); +create table rules_log(f1 int, f2 int, tag text); +insert into rules_src values(1,2), (11,12); +create rule r1 as on update to rules_src do also + insert into rules_log values(old.*, 'old'), (new.*, 'new'); +update rules_src set f2 = f2 + 1; +update rules_src set f2 = f2 * 10; +select * from rules_src; + f1 | f2 +----+----- + 1 | 30 + 11 | 130 +(2 rows) + +select * from rules_log; + f1 | f2 | tag +----+-----+----- + 1 | 2 | old + 1 | 3 | new + 11 | 12 | old + 11 | 13 | new + 1 | 3 | old + 1 | 30 | new + 11 | 13 | old + 11 | 130 | new +(8 rows) + +create rule r2 as on update to rules_src do also + values(old.*, 'old'), (new.*, 'new'); +update rules_src set f2 = f2 / 10; + column1 | column2 | column3 +---------+---------+--------- + 1 | 30 | old + 1 | 3 | new + 11 | 130 | old + 11 | 13 | new +(4 rows) + +select * from rules_src; + f1 | f2 +----+---- + 1 | 3 + 11 | 13 +(2 rows) + +select * from rules_log; + f1 | f2 | tag +----+-----+----- + 1 | 2 | old + 1 | 3 | new + 11 | 12 | old + 11 | 13 | new + 1 | 3 | old + 1 | 30 | new + 11 | 13 | old + 11 | 130 | new + 1 | 30 | old + 1 | 3 | new + 11 | 130 | old + 11 | 13 | new +(12 rows) + +\d+ rules_src + Table "public.rules_src" + Column | Type | Modifiers | Storage | Stats target | Description +--------+---------+-----------+---------+--------------+------------- + f1 | integer | | plain | | + f2 | integer | | plain | | +Rules: + r1 AS + ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag) VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text) + r2 AS + ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text) +Has OIDs: no + diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index ea665a3c8b..7bc48c170e 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -933,3 +933,23 @@ select * from only t1_2; select pg_get_viewdef('shoe'::regclass) as unpretty; select pg_get_viewdef('shoe'::regclass,true) as pretty; select pg_get_viewdef('shoe'::regclass,0) as prettier; + +-- +-- check multi-rule VALUES in rules +-- + +create table rules_src(f1 int, f2 int); +create table rules_log(f1 int, f2 int, tag text); +insert into rules_src values(1,2), (11,12); +create rule r1 as on update to rules_src do also + insert into rules_log values(old.*, 'old'), (new.*, 'new'); +update rules_src set f2 = f2 + 1; +update rules_src set f2 = f2 * 10; +select * from rules_src; +select * from rules_log; +create rule r2 as on update to rules_src do also + values(old.*, 'old'), (new.*, 'new'); +update rules_src set f2 = f2 / 10; +select * from rules_src; +select * from rules_log; +\d+ rules_src -- 2.40.0