diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index b1cede2ef0..9392d61dba 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -1935,16 +1935,21 @@ search_indexed_tlist_for_sortgroupref(Node *node, * relation target lists. Also perform opcode lookup and add * regclass OIDs to root->glob->relationOids. * - * This is used in two different scenarios: a normal join clause, where all - * the Vars in the clause *must* be replaced by OUTER_VAR or INNER_VAR - * references; and a RETURNING clause, which may contain both Vars of the - * target relation and Vars of other relations. In the latter case we want - * to replace the other-relation Vars by OUTER_VAR references, while leaving - * target Vars alone. - * - * For a normal join, acceptable_rel should be zero so that any failure to - * match a Var will be reported as an error. For the RETURNING case, pass - * inner_itlist = NULL and acceptable_rel = the ID of the target relation. + * This is used in three different scenarios: + * 1) a normal join clause, where all the Vars in the clause *must* be + * replaced by OUTER_VAR or INNER_VAR references. In this case + * acceptable_rel should be zero so that any failure to match a Var will be + * reported as an error. + * 2) RETURNING clauses, which may contain both Vars of the target relation + * and Vars of other relations. In this case we want to replace the + * other-relation Vars by OUTER_VAR references, while leaving target Vars + * alone. Thus inner_itlist = NULL and acceptable_rel = the ID of the + * target relation should be passed. + * 3) ON CONFLICT UPDATE SET/WHERE clauses. Here references to EXCLUDED are + * to be replaced with INNER_VAR references, while leaving target Vars (the + * to-be-updated relation) alone. Correspondingly inner_itlist is to be + * EXCLUDED elements, outer_itlist = NULL and acceptable_rel the target + * relation. * * 'clauses' is the targetlist or list of join clauses * 'outer_itlist' is the indexed target list of the outer join relation, @@ -1987,7 +1992,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) { Var *var = (Var *) node; - /* First look for the var in the input tlists */ + /* Look for the var in the input tlists, first in the outer */ if (context->outer_itlist) { newvar = search_indexed_tlist_for_var(var, @@ -1998,7 +2003,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) return (Node *) newvar; } - /* Then in the outer */ + /* then in the inner. */ if (context->inner_itlist) { newvar = search_indexed_tlist_for_var(var, diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index a0dfbf900a..3ecb790ceb 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -891,33 +891,81 @@ transformOnConflictClause(ParseState *pstate, /* Process DO UPDATE */ if (onConflictClause->action == ONCONFLICT_UPDATE) { + Relation targetrel = pstate->p_target_relation; + Var *var; + TargetEntry *te; + int attno; + /* * All INSERT expressions have been parsed, get ready for potentially * existing SET statements that need to be processed like an UPDATE. */ pstate->p_is_insert = false; + /* + * Add range table entry for the EXCLUDED pseudo relation; relkind is + * set to composite to signal that we're not dealing with an actual + * relation. + */ exclRte = addRangeTableEntryForRelation(pstate, - pstate->p_target_relation, + targetrel, makeAlias("excluded", NIL), false, false); + exclRte->relkind = RELKIND_COMPOSITE_TYPE; exclRelIndex = list_length(pstate->p_rtable); /* - * Build a targetlist for the EXCLUDED pseudo relation. Out of - * simplicity we do that here, because expandRelAttrs() happens to - * nearly do the right thing; specifically it also works with views. - * It'd be more proper to instead scan some pseudo scan node, but it - * doesn't seem worth the amount of code required. - * - * The only caveat of this hack is that the permissions expandRelAttrs - * adds have to be reset. markVarForSelectPriv() will add the exact - * required permissions back. + * Build a targetlist for the EXCLUDED pseudo relation. Have to be + * careful to use resnos that correspond to attnos of the underlying + * relation. */ - exclRelTlist = expandRelAttrs(pstate, exclRte, - exclRelIndex, 0, -1); - exclRte->requiredPerms = 0; - exclRte->selectedCols = NULL; + Assert(pstate->p_next_resno == 1); + for (attno = 0; attno < targetrel->rd_rel->relnatts; attno++) + { + Form_pg_attribute attr = targetrel->rd_att->attrs[attno]; + char *name; + + if (attr->attisdropped) + { + /* + * can't use atttypid here, but it doesn't really matter what + * type the Const claims to be. + */ + var = (Var *) makeNullConst(INT4OID, -1, InvalidOid); + name = ""; + } + else + { + var = makeVar(exclRelIndex, attno + 1, + attr->atttypid, attr->atttypmod, + attr->attcollation, + 0); + var->location = -1; + + name = NameStr(attr->attname); + } + + Assert(pstate->p_next_resno == attno + 1); + te = makeTargetEntry((Expr *) var, + pstate->p_next_resno++, + name, + false); + + /* don't require select access yet */ + exclRelTlist = lappend(exclRelTlist, te); + } + + /* + * Additionally add a whole row tlist entry for EXCLUDED. That's + * really only needed for ruleutils' benefit, which expects to find + * corresponding entries in child tlists. Alternatively we could do + * this only when required, but that doesn't seem worth the trouble. + */ + var = makeVar(exclRelIndex, InvalidAttrNumber, + RelationGetRelid(targetrel), + -1, InvalidOid, 0); + te = makeTargetEntry((Expr *) var, 0, NULL, true); + exclRelTlist = lappend(exclRelTlist, te); /* * Add EXCLUDED and the target RTE to the namespace, so that they can diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 0b2dacfd59..0c4ed65afa 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -686,9 +686,12 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, char *colname, return result; /* - * If the RTE represents a real table, consider system column names. + * If the RTE represents a real relation, consider system column names. + * Composites are only used for pseudo-relations like ON CONFLICT's + * excluded. */ - if (rte->rtekind == RTE_RELATION) + if (rte->rtekind == RTE_RELATION && + rte->relkind != RELKIND_COMPOSITE_TYPE) { /* quick check to see if name could be a system column */ attnum = specialAttNum(colname); diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 1399f3c796..44bc01bf5a 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -380,8 +380,80 @@ ERROR: there is no unique or exclusion constraint matching the ON CONFLICT spec insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) where fruit like '%berry' do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification drop index partial_key_index; +-- +-- Test that wholerow references to ON CONFLICT's EXCLUDED work +-- +create unique index plain on insertconflicttest(key); +-- Succeeds, updates existing row: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* != excluded.* returning *; + key | fruit +-----+----------- + 23 | Jackfruit +(1 row) + +-- No update this time, though: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* != excluded.* returning *; + key | fruit +-----+------- +(0 rows) + +-- Predicate changed to require match rather than non-match, so updates once more: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* = excluded.* returning *; + key | fruit +-----+----------- + 23 | Jackfruit +(1 row) + +-- Assign: +insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text + returning *; + key | fruit +-----+-------------- + 23 | (23,Avocado) +(1 row) + +-- deparse whole row var in WHERE and SET clauses: +explain (costs off) insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.fruit where excluded.* is null; + QUERY PLAN +----------------------------------------- + Insert on insertconflicttest i + Conflict Resolution: UPDATE + Conflict Arbiter Indexes: plain + Conflict Filter: (excluded.* IS NULL) + -> Result +(5 rows) + +explain (costs off) insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text; + QUERY PLAN +----------------------------------- + Insert on insertconflicttest i + Conflict Resolution: UPDATE + Conflict Arbiter Indexes: plain + -> Result +(4 rows) + +drop index plain; -- Cleanup drop table insertconflicttest; +-- +-- Verify that EXCLUDED does not allow system column references. These +-- do not make sense because EXCLUDED isn't an already stored tuple +-- (and thus doesn't have a ctid, oids are not assigned yet, etc). +-- +create table syscolconflicttest(key int4, data text) WITH OIDS; +insert into syscolconflicttest values (1); +insert into syscolconflicttest values (1) on conflict (key) do update set data = excluded.ctid::text; +ERROR: column excluded.ctid does not exist +LINE 1: ...values (1) on conflict (key) do update set data = excluded.c... + ^ +insert into syscolconflicttest values (1) on conflict (key) do update set data = excluded.oid::text; +ERROR: column excluded.oid does not exist +LINE 1: ...values (1) on conflict (key) do update set data = excluded.o... + ^ +drop table syscolconflicttest; -- ****************************************************************** -- * * -- * Test inheritance (example taken from tutorial) * @@ -566,3 +638,52 @@ insert into testoids values(3, '2') on conflict (key) do update set data = exclu (1 row) DROP TABLE testoids; +-- check that references to columns after dropped columns are handled correctly +create table dropcol(key int primary key, drop1 int, keep1 text, drop2 numeric, keep2 float); +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 1, '1', '1', 1); +-- set using excluded +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 2, '2', '2', 2) on conflict(key) + do update set drop1 = excluded.drop1, keep1 = excluded.keep1, drop2 = excluded.drop2, keep2 = excluded.keep2 + where excluded.drop1 is not null and excluded.keep1 is not null and excluded.drop2 is not null and excluded.keep2 is not null + and dropcol.drop1 is not null and dropcol.keep1 is not null and dropcol.drop2 is not null and dropcol.keep2 is not null + returning *; + key | drop1 | keep1 | drop2 | keep2 +-----+-------+-------+-------+------- + 1 | 2 | 2 | 2 | 2 +(1 row) + +; +-- set using existing table +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 3, '3', '3', 3) on conflict(key) + do update set drop1 = dropcol.drop1, keep1 = dropcol.keep1, drop2 = dropcol.drop2, keep2 = dropcol.keep2 + returning *; + key | drop1 | keep1 | drop2 | keep2 +-----+-------+-------+-------+------- + 1 | 2 | 2 | 2 | 2 +(1 row) + +; +alter table dropcol drop column drop1, drop column drop2; +-- set using excluded +insert into dropcol(key, keep1, keep2) values(1, '4', 4) on conflict(key) + do update set keep1 = excluded.keep1, keep2 = excluded.keep2 + where excluded.keep1 is not null and excluded.keep2 is not null + and dropcol.keep1 is not null and dropcol.keep2 is not null + returning *; + key | keep1 | keep2 +-----+-------+------- + 1 | 4 | 4 +(1 row) + +; +-- set using existing table +insert into dropcol(key, keep1, keep2) values(1, '5', 5) on conflict(key) + do update set keep1 = dropcol.keep1, keep2 = dropcol.keep2 + returning *; + key | keep1 | keep2 +-----+-------+------- + 1 | 4 | 4 +(1 row) + +; +DROP TABLE dropcol; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 44c6740582..80374e4d50 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2900,7 +2900,7 @@ CREATE RULE hat_upsert AS ON INSERT TO hats ON CONFLICT (hat_name) DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color - WHERE excluded.hat_color <> 'forbidden' + WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.* RETURNING *; SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename; definition @@ -2908,7 +2908,7 @@ SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename; CREATE RULE hat_upsert AS + ON INSERT TO hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) + VALUES (new.hat_name, new.hat_color) ON CONFLICT(hat_name) DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color+ - WHERE (excluded.hat_color <> 'forbidden'::bpchar) + + WHERE ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*)) + RETURNING hat_data.hat_name, + hat_data.hat_color; (1 row) @@ -2956,19 +2956,19 @@ SELECT tablename, rulename, definition FROM pg_rules hats | hat_upsert | CREATE RULE hat_upsert AS + | | ON INSERT TO hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) + | | VALUES (new.hat_name, new.hat_color) ON CONFLICT(hat_name) DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color+ - | | WHERE (excluded.hat_color <> 'forbidden'::bpchar) + + | | WHERE ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*)) + | | RETURNING hat_data.hat_name, + | | hat_data.hat_color; (1 row) -- ensure explain works for on insert conflict rules explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *; - QUERY PLAN ----------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------- Insert on hat_data Conflict Resolution: UPDATE Conflict Arbiter Indexes: hat_data_unique_idx - Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar) + Conflict Filter: ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*)) -> Result (5 rows) @@ -2995,12 +2995,12 @@ EXPLAIN (costs off) WITH data(hat_name, hat_color) AS ( INSERT INTO hats SELECT * FROM data RETURNING *; - QUERY PLAN ----------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------- Insert on hat_data Conflict Resolution: UPDATE Conflict Arbiter Indexes: hat_data_unique_idx - Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar) + Conflict Filter: ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*)) CTE data -> Values Scan on "*VALUES*" -> CTE Scan on data diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index efa902ec12..8d846f51df 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -223,9 +223,45 @@ insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) whe drop index partial_key_index; +-- +-- Test that wholerow references to ON CONFLICT's EXCLUDED work +-- +create unique index plain on insertconflicttest(key); + +-- Succeeds, updates existing row: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* != excluded.* returning *; +-- No update this time, though: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* != excluded.* returning *; +-- Predicate changed to require match rather than non-match, so updates once more: +insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit + where i.* = excluded.* returning *; +-- Assign: +insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text + returning *; +-- deparse whole row var in WHERE and SET clauses: +explain (costs off) insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.fruit where excluded.* is null; +explain (costs off) insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text; + +drop index plain; + -- Cleanup drop table insertconflicttest; + +-- +-- Verify that EXCLUDED does not allow system column references. These +-- do not make sense because EXCLUDED isn't an already stored tuple +-- (and thus doesn't have a ctid, oids are not assigned yet, etc). +-- +create table syscolconflicttest(key int4, data text) WITH OIDS; +insert into syscolconflicttest values (1); +insert into syscolconflicttest values (1) on conflict (key) do update set data = excluded.ctid::text; +insert into syscolconflicttest values (1) on conflict (key) do update set data = excluded.oid::text; +drop table syscolconflicttest; + + -- ****************************************************************** -- * * -- * Test inheritance (example taken from tutorial) * @@ -317,3 +353,35 @@ insert into testoids values(3, '1') on conflict (key) do update set data = exclu insert into testoids values(3, '2') on conflict (key) do update set data = excluded.data RETURNING *; DROP TABLE testoids; + + +-- check that references to columns after dropped columns are handled correctly +create table dropcol(key int primary key, drop1 int, keep1 text, drop2 numeric, keep2 float); +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 1, '1', '1', 1); +-- set using excluded +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 2, '2', '2', 2) on conflict(key) + do update set drop1 = excluded.drop1, keep1 = excluded.keep1, drop2 = excluded.drop2, keep2 = excluded.keep2 + where excluded.drop1 is not null and excluded.keep1 is not null and excluded.drop2 is not null and excluded.keep2 is not null + and dropcol.drop1 is not null and dropcol.keep1 is not null and dropcol.drop2 is not null and dropcol.keep2 is not null + returning *; +; +-- set using existing table +insert into dropcol(key, drop1, keep1, drop2, keep2) values(1, 3, '3', '3', 3) on conflict(key) + do update set drop1 = dropcol.drop1, keep1 = dropcol.keep1, drop2 = dropcol.drop2, keep2 = dropcol.keep2 + returning *; +; +alter table dropcol drop column drop1, drop column drop2; +-- set using excluded +insert into dropcol(key, keep1, keep2) values(1, '4', 4) on conflict(key) + do update set keep1 = excluded.keep1, keep2 = excluded.keep2 + where excluded.keep1 is not null and excluded.keep2 is not null + and dropcol.keep1 is not null and dropcol.keep2 is not null + returning *; +; +-- set using existing table +insert into dropcol(key, keep1, keep2) values(1, '5', 5) on conflict(key) + do update set keep1 = dropcol.keep1, keep2 = dropcol.keep2 + returning *; +; + +DROP TABLE dropcol; diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 561e2fd29a..4299a5b172 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1105,7 +1105,7 @@ CREATE RULE hat_upsert AS ON INSERT TO hats ON CONFLICT (hat_name) DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color - WHERE excluded.hat_color <> 'forbidden' + WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.* RETURNING *; SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename;