diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index de9aaf3f76..66cac46acf 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1126,10 +1126,10 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation) /* * Okay only if there's a suitable INSTEAD OF trigger. Messages - * here should match rewriteHandler.c's rewriteTargetView, except - * that we omit errdetail because we haven't got the information - * handy (and given that we really shouldn't get here anyway, it's - * not worth great exertion to get). + * here should match rewriteHandler.c's rewriteTargetView and + * RewriteQuery, except that we omit errdetail because we haven't + * got the information handy (and given that we really shouldn't + * get here anyway, it's not worth great exertion to get). */ switch (operation) { diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 04b79fd1d0..ebb29e9aa3 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -3630,21 +3630,71 @@ RewriteQuery(Query *parsetree, List *rewrite_events) } /* - * If there were no INSTEAD rules, and the target relation is a view - * without any INSTEAD OF triggers, see if the view can be + * If there was no unqualified INSTEAD rule, and the target relation + * is a view without any INSTEAD OF triggers, see if the view can be * automatically updated. If so, we perform the necessary query * transformation here and add the resulting query to the * product_queries list, so that it gets recursively rewritten if * necessary. + * + * If the view cannot be automatically updated, we throw an error here + * which is OK since the query would fail at runtime anyway. Throwing + * the error here is preferable to the executor check since we have + * more detailed information available about why the view isn't + * updatable. */ - if (!instead && qual_product == NULL && + if (!instead && rt_entry_relation->rd_rel->relkind == RELKIND_VIEW && !view_has_instead_trigger(rt_entry_relation, event)) { /* + * If there were any qualified INSTEAD rules, don't allow the view + * to be automatically updated (an unqualified INSTEAD rule or + * INSTEAD OF trigger is required). + * + * The messages here should match execMain.c's CheckValidResultRel + * and in principle make those checks in executor unnecessary, but + * we keep them just in case. + */ + if (qual_product != NULL) + { + switch (parsetree->commandType) + { + case CMD_INSERT: + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot insert into view \"%s\"", + RelationGetRelationName(rt_entry_relation)), + errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."), + errhint("To enable inserting into the view, provide an INSTEAD OF INSERT trigger or an unconditional ON INSERT DO INSTEAD rule."))); + break; + case CMD_UPDATE: + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot update view \"%s\"", + RelationGetRelationName(rt_entry_relation)), + errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."), + errhint("To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule."))); + break; + case CMD_DELETE: + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot delete from view \"%s\"", + RelationGetRelationName(rt_entry_relation)), + errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."), + errhint("To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule."))); + break; + default: + elog(ERROR, "unrecognized CmdType: %d", + (int) parsetree->commandType); + break; + } + } + + /* + * Attempt to rewrite the query to automatically update the view. * This throws an error if the view can't be automatically - * updated, but that's OK since the query would fail at runtime - * anyway. + * updated. */ parsetree = rewriteTargetView(parsetree, rt_entry_relation); diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 300a03bd70..f49c8d035b 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -328,6 +328,27 @@ UPDATE ro_view20 SET b=upper(b); ERROR: cannot update view "ro_view20" DETAIL: Views that return set-returning functions are not automatically updatable. HINT: To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule. +-- A view with a conditional INSTEAD rule but no unconditional INSTEAD rules +-- or INSTEAD OF triggers should be non-updatable and generate useful error +-- messages with appropriate detail +CREATE RULE rw_view16_ins_rule AS ON INSERT TO rw_view16 + WHERE NEW.a > 0 DO INSTEAD INSERT INTO base_tbl VALUES (NEW.a, NEW.b); +CREATE RULE rw_view16_upd_rule AS ON UPDATE TO rw_view16 + WHERE OLD.a > 0 DO INSTEAD UPDATE base_tbl SET b=NEW.b WHERE a=OLD.a; +CREATE RULE rw_view16_del_rule AS ON DELETE TO rw_view16 + WHERE OLD.a > 0 DO INSTEAD DELETE FROM base_tbl WHERE a=OLD.a; +INSERT INTO rw_view16 (a, b) VALUES (3, 'Row 3'); -- should fail +ERROR: cannot insert into view "rw_view16" +DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable. +HINT: To enable inserting into the view, provide an INSTEAD OF INSERT trigger or an unconditional ON INSERT DO INSTEAD rule. +UPDATE rw_view16 SET b='ROW 2' WHERE a=2; -- should fail +ERROR: cannot update view "rw_view16" +DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable. +HINT: To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule. +DELETE FROM rw_view16 WHERE a=2; -- should fail +ERROR: cannot delete from view "rw_view16" +DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable. +HINT: To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule. DROP TABLE base_tbl CASCADE; NOTICE: drop cascades to 16 other objects DETAIL: drop cascades to view ro_view1 diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql index e928e86ca3..77a97191a9 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -98,6 +98,20 @@ DELETE FROM ro_view18; UPDATE ro_view19 SET last_value=1000; UPDATE ro_view20 SET b=upper(b); +-- A view with a conditional INSTEAD rule but no unconditional INSTEAD rules +-- or INSTEAD OF triggers should be non-updatable and generate useful error +-- messages with appropriate detail +CREATE RULE rw_view16_ins_rule AS ON INSERT TO rw_view16 + WHERE NEW.a > 0 DO INSTEAD INSERT INTO base_tbl VALUES (NEW.a, NEW.b); +CREATE RULE rw_view16_upd_rule AS ON UPDATE TO rw_view16 + WHERE OLD.a > 0 DO INSTEAD UPDATE base_tbl SET b=NEW.b WHERE a=OLD.a; +CREATE RULE rw_view16_del_rule AS ON DELETE TO rw_view16 + WHERE OLD.a > 0 DO INSTEAD DELETE FROM base_tbl WHERE a=OLD.a; + +INSERT INTO rw_view16 (a, b) VALUES (3, 'Row 3'); -- should fail +UPDATE rw_view16 SET b='ROW 2' WHERE a=2; -- should fail +DELETE FROM rw_view16 WHERE a=2; -- should fail + DROP TABLE base_tbl CASCADE; DROP VIEW ro_view10, ro_view12, ro_view18; DROP SEQUENCE uv_seq CASCADE;