From 9bc77c45199c7d2e525cd5b1457d5a57f6e9edb0 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 19 May 2015 21:07:28 +0200 Subject: [PATCH] Various fixes around ON CONFLICT for rule deparsing. Neither the deparsing of the new alias for INSERT's target table, nor of the inference clause was supported. Also fixup a typo in an error message. Add regression tests to test those code paths. Author: Peter Geoghegan --- src/backend/parser/parse_clause.c | 2 +- src/backend/utils/adt/ruleutils.c | 81 ++++++++++++++++++- src/test/regress/expected/insert_conflict.out | 2 +- src/test/regress/expected/rules.out | 66 +++++++++++---- src/test/regress/sql/rules.sql | 14 +++- 5 files changed, 141 insertions(+), 24 deletions(-) diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index c8af5ab1d0..f8eebfe8c3 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -2765,7 +2765,7 @@ transformOnConflictArbiter(ParseState *pstate, ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("ON CONFLICT DO UPDATE requires inference specification or constraint name"), - errhint("For example, ON CONFLICT ON CONFLICT ()."), + errhint("For example, ON CONFLICT ()."), parser_errposition(pstate, exprLocation((Node *) onConflictClause)))); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 0a77400a80..8cdef086a0 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -5392,6 +5392,10 @@ get_insert_query_def(Query *query, deparse_context *context) } appendStringInfo(buf, "INSERT INTO %s ", generate_relation_name(rte->relid, NIL)); + /* INSERT requires AS keyword for target alias */ + if (rte->alias != NULL) + appendStringInfo(buf, "AS %s ", + quote_identifier(rte->alias->aliasname)); /* * Add the insert-column-names list. To handle indirection properly, we @@ -5479,13 +5483,38 @@ get_insert_query_def(Query *query, deparse_context *context) { OnConflictExpr *confl = query->onConflict; - if (confl->action == ONCONFLICT_NOTHING) + appendStringInfo(buf, " ON CONFLICT"); + + if (confl->arbiterElems) { - appendStringInfoString(buf, " ON CONFLICT DO NOTHING"); + /* Add the single-VALUES expression list */ + appendStringInfoChar(buf, '('); + get_rule_expr((Node *) confl->arbiterElems, context, false); + appendStringInfoChar(buf, ')'); + + /* Add a WHERE clause (for partial indexes) if given */ + if (confl->arbiterWhere != NULL) + { + appendContextKeyword(context, " WHERE ", + -PRETTYINDENT_STD, PRETTYINDENT_STD, 1); + get_rule_expr(confl->arbiterWhere, context, false); + } } else { - appendStringInfoString(buf, " ON CONFLICT DO UPDATE SET "); + char *constraint = get_constraint_name(confl->constraint); + + appendStringInfo(buf, " ON CONSTRAINT %s", + quote_qualified_identifier(NULL, constraint)); + } + + if (confl->action == ONCONFLICT_NOTHING) + { + appendStringInfoString(buf, " DO NOTHING"); + } + else + { + appendStringInfoString(buf, " DO UPDATE SET "); /* Deparse targetlist */ get_update_query_targetlist_def(query, confl->onConflictSet, context, rte); @@ -7886,6 +7915,52 @@ get_rule_expr(Node *node, deparse_context *context, } break; + case T_InferenceElem: + { + InferenceElem *iexpr = (InferenceElem *) node; + bool varprefix = context->varprefix; + bool need_parens; + + /* + * InferenceElem can only refer to target relation, so a + * prefix is never useful. + */ + context->varprefix = false; + + /* + * Parenthesize the element unless it's a simple Var or a bare + * function call. Follows pg_get_indexdef_worker(). + */ + need_parens = !IsA(iexpr->expr, Var); + if (IsA(iexpr->expr, FuncExpr) && + ((FuncExpr *) iexpr->expr)->funcformat == + COERCE_EXPLICIT_CALL) + need_parens = false; + + if (need_parens) + appendStringInfoChar(buf, '('); + get_rule_expr((Node *) iexpr->expr, + context, false); + if (need_parens) + appendStringInfoChar(buf, ')'); + + context->varprefix = varprefix; + + if (iexpr->infercollid) + appendStringInfo(buf, " COLLATE %s", + generate_collation_name(iexpr->infercollid)); + + /* Add the operator class name, if not default */ + if (iexpr->inferopclass) + { + Oid inferopclass = iexpr->inferopclass; + Oid inferopcinputtype = get_opclass_input_type(iexpr->inferopclass); + + get_opclass_name(inferopclass, inferopcinputtype, buf); + } + } + break; + case T_List: { char *sep; diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 8e4e33e6e6..eca9690592 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -208,7 +208,7 @@ insert into insertconflicttest values (1, 'Apple') on conflict do update set fru ERROR: ON CONFLICT DO UPDATE requires inference specification or constraint name LINE 1: ...nsert into insertconflicttest values (1, 'Apple') on conflic... ^ -HINT: For example, ON CONFLICT ON CONFLICT (). +HINT: For example, ON CONFLICT (). -- inference succeeds: insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit; insert into insertconflicttest values (2, 'Orange') on conflict (key, key, key) do update set fruit = excluded.fruit; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 6eb2e8c8b7..a2ed8fa7fa 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2676,6 +2676,34 @@ Rules: ON DELETE TO rules_src DO NOTIFY rules_src_deletion +-- +-- Ensure a aliased target relation for insert is correctly deparsed. +-- +create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; +create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; +\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) + r3 AS + ON DELETE TO rules_src DO + NOTIFY rules_src_deletion + r4 AS + ON INSERT TO rules_src DO INSTEAD INSERT INTO rules_log AS trgt (f1, f2) SELECT new.f1, + new.f2 + RETURNING trgt.f1, + trgt.f2 + r5 AS + ON UPDATE TO rules_src DO INSTEAD UPDATE rules_log trgt SET tag = 'updated'::text + WHERE trgt.f1 = new.f1 + -- -- check alter rename rule -- @@ -2778,16 +2806,19 @@ CREATE TABLE hats ( hat_color char(10) -- hat color ); CREATE TABLE hat_data ( - hat_name char(10) primary key, + hat_name char(10), hat_color char(10) -- hat color ); +create unique index hat_data_unique_idx + on hat_data (hat_name COLLATE "C" bpchar_pattern_ops); -- okay CREATE RULE hat_nosert AS ON INSERT TO hats DO INSTEAD INSERT INTO hat_data VALUES ( NEW.hat_name, NEW.hat_color) - ON CONFLICT (hat_name) DO NOTHING RETURNING *; + ON CONFLICT (hat_name COLLATE "C" bpchar_pattern_ops) WHERE hat_color = 'green' + DO NOTHING RETURNING *; -- Works (projects row) INSERT INTO hats VALUES ('h7', 'black') RETURNING *; hat_name | hat_color @@ -2803,12 +2834,13 @@ INSERT INTO hats VALUES ('h7', 'black') RETURNING *; SELECT tablename, rulename, definition FROM pg_rules WHERE tablename = 'hats'; - tablename | rulename | definition ------------+------------+------------------------------------------------------------------------------ - hats | hat_nosert | CREATE RULE hat_nosert AS + - | | ON INSERT TO hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color)+ - | | VALUES (new.hat_name, new.hat_color) ON CONFLICT DO NOTHING + - | | RETURNING hat_data.hat_name, + + tablename | rulename | definition +-----------+------------+--------------------------------------------------------------------------------------------- + hats | hat_nosert | CREATE RULE hat_nosert 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 COLLATE "C" bpchar_pattern_ops)+ + | | WHERE (hat_data.hat_color = 'green'::bpchar) DO NOTHING + + | | RETURNING hat_data.hat_name, + | | hat_data.hat_color; (1 row) @@ -2861,13 +2893,13 @@ SELECT * FROM hat_data WHERE hat_name = 'h8'; SELECT tablename, rulename, definition FROM pg_rules WHERE tablename = 'hats'; - tablename | rulename | definition ------------+------------+------------------------------------------------------------------------------------------------------------------------------- - 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 DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color+ - | | WHERE (excluded.hat_color <> 'forbidden'::bpchar) + - | | RETURNING hat_data.hat_name, + + tablename | rulename | definition +-----------+------------+----------------------------------------------------------------------------------------------------------------------------------------- + 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) + + | | RETURNING hat_data.hat_name, + | | hat_data.hat_color; (1 row) @@ -2877,7 +2909,7 @@ explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *; ---------------------------------------------------------------- Insert on hat_data Conflict Resolution: UPDATE - Conflict Arbiter Indexes: hat_data_pkey + Conflict Arbiter Indexes: hat_data_unique_idx Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar) -> Result (5 rows) @@ -2909,7 +2941,7 @@ RETURNING *; ---------------------------------------------------------------- Insert on hat_data Conflict Resolution: UPDATE - Conflict Arbiter Indexes: hat_data_pkey + Conflict Arbiter Indexes: hat_data_unique_idx Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar) CTE data -> Values Scan on "*VALUES*" diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 1a81155bf1..1f9f7e69d2 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -996,6 +996,13 @@ select * from rules_log; create rule r3 as on delete to rules_src do notify rules_src_deletion; \d+ rules_src +-- +-- Ensure a aliased target relation for insert is correctly deparsed. +-- +create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; +create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; +\d+ rules_src + -- -- check alter rename rule -- @@ -1049,9 +1056,11 @@ CREATE TABLE hats ( ); CREATE TABLE hat_data ( - hat_name char(10) primary key, + hat_name char(10), hat_color char(10) -- hat color ); +create unique index hat_data_unique_idx + on hat_data (hat_name COLLATE "C" bpchar_pattern_ops); -- okay CREATE RULE hat_nosert AS ON INSERT TO hats @@ -1059,7 +1068,8 @@ CREATE RULE hat_nosert AS ON INSERT TO hats INSERT INTO hat_data VALUES ( NEW.hat_name, NEW.hat_color) - ON CONFLICT (hat_name) DO NOTHING RETURNING *; + ON CONFLICT (hat_name COLLATE "C" bpchar_pattern_ops) WHERE hat_color = 'green' + DO NOTHING RETURNING *; -- Works (projects row) INSERT INTO hats VALUES ('h7', 'black') RETURNING *;