Skip to content

Commit 9be6fcb

Browse files
deanrasheeddeanrasheed
committed
Make rewriter prevent auto-updates on views with conditional INSTEAD rules.
A view with conditional INSTEAD rules and no unconditional INSTEAD rules or INSTEAD OF triggers is not auto-updatable. Previously we relied on a check in the executor to catch this, but that's problematic since the planner may fail to properly handle such a query and thus return a particularly unhelpful error to the user, before reaching the executor check. Instead, trap this in the rewriter and report the correct error there. Doing so also allows us to include more useful error detail than the executor check can provide. This doesn't change the existing behaviour of updatable views; it merely ensures that useful error messages are reported when a view isn't updatable. Per report from Pengzhou Tang, though not adopting that suggested fix. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAG4reAQn+4xB6xHJqWdtE0ve_WqJkdyCV4P=trYr4Kn8_3_PEA@mail.gmail.com
1 parent 6bd567b commit 9be6fcb

File tree

4 files changed

+94
-9
lines changed

4 files changed

+94
-9
lines changed

‎src/backend/executor/execMain.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -981,10 +981,10 @@ CheckValidResultRel(Relation resultRel, CmdType operation)
981981

982982
/*
983983
* Okay only if there's a suitable INSTEAD OF trigger. Messages
984-
* here should match rewriteHandler.c's rewriteTargetView, except
985-
* that we omit errdetail because we haven't got the information
986-
* handy (and given that we really shouldn't get here anyway, it's
987-
* not worth great exertion to get).
984+
* here should match rewriteHandler.c's rewriteTargetView and
985+
* RewriteQuery, except that we omit errdetail because we haven't
986+
* got the information handy (and given that we really shouldn't
987+
* get here anyway, it's not worth great exertion to get).
988988
*/
989989
switch (operation)
990990
{

‎src/backend/rewrite/rewriteHandler.c

+55-5
Original file line numberDiff line numberDiff line change
@@ -3301,21 +3301,71 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
33013301
}
33023302

33033303
/*
3304-
* If there were no INSTEAD rules, and the target relation is a view
3305-
* without any INSTEAD OF triggers, see if the view can be
3304+
* If there was no unqualified INSTEAD rule, and the target relation
3305+
* is a view without any INSTEAD OF triggers, see if the view can be
33063306
* automatically updated. If so, we perform the necessary query
33073307
* transformation here and add the resulting query to the
33083308
* product_queries list, so that it gets recursively rewritten if
33093309
* necessary.
3310+
*
3311+
* If the view cannot be automatically updated, we throw an error here
3312+
* which is OK since the query would fail at runtime anyway. Throwing
3313+
* the error here is preferable to the executor check since we have
3314+
* more detailed information available about why the view isn't
3315+
* updatable.
33103316
*/
3311-
if (!instead && qual_product == NULL &&
3317+
if (!instead &&
33123318
rt_entry_relation->rd_rel->relkind == RELKIND_VIEW &&
33133319
!view_has_instead_trigger(rt_entry_relation, event))
33143320
{
33153321
/*
3322+
* If there were any qualified INSTEAD rules, don't allow the view
3323+
* to be automatically updated (an unqualified INSTEAD rule or
3324+
* INSTEAD OF trigger is required).
3325+
*
3326+
* The messages here should match execMain.c's CheckValidResultRel
3327+
* and in principle make those checks in executor unnecessary, but
3328+
* we keep them just in case.
3329+
*/
3330+
if (qual_product != NULL)
3331+
{
3332+
switch (parsetree->commandType)
3333+
{
3334+
case CMD_INSERT:
3335+
ereport(ERROR,
3336+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
3337+
errmsg("cannot insert into view \"%s\"",
3338+
RelationGetRelationName(rt_entry_relation)),
3339+
errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."),
3340+
errhint("To enable inserting into the view, provide an INSTEAD OF INSERT trigger or an unconditional ON INSERT DO INSTEAD rule.")));
3341+
break;
3342+
case CMD_UPDATE:
3343+
ereport(ERROR,
3344+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
3345+
errmsg("cannot update view \"%s\"",
3346+
RelationGetRelationName(rt_entry_relation)),
3347+
errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."),
3348+
errhint("To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule.")));
3349+
break;
3350+
case CMD_DELETE:
3351+
ereport(ERROR,
3352+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
3353+
errmsg("cannot delete from view \"%s\"",
3354+
RelationGetRelationName(rt_entry_relation)),
3355+
errdetail("Views with conditional DO INSTEAD rules are not automatically updatable."),
3356+
errhint("To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule.")));
3357+
break;
3358+
default:
3359+
elog(ERROR, "unrecognized CmdType: %d",
3360+
(int) parsetree->commandType);
3361+
break;
3362+
}
3363+
}
3364+
3365+
/*
3366+
* Attempt to rewrite the query to automatically update the view.
33163367
* This throws an error if the view can't be automatically
3317-
* updated, but that's OK since the query would fail at runtime
3318-
* anyway.
3368+
* updated.
33193369
*/
33203370
parsetree = rewriteTargetView(parsetree, rt_entry_relation);
33213371

‎src/test/regress/expected/updatable_views.out

+21
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,27 @@ UPDATE ro_view20 SET b=upper(b);
274274
ERROR: cannot update view "ro_view20"
275275
DETAIL: Views that return set-returning functions are not automatically updatable.
276276
HINT: To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule.
277+
-- A view with a conditional INSTEAD rule but no unconditional INSTEAD rules
278+
-- or INSTEAD OF triggers should be non-updatable and generate useful error
279+
-- messages with appropriate detail
280+
CREATE RULE rw_view16_ins_rule AS ON INSERT TO rw_view16
281+
WHERE NEW.a > 0 DO INSTEAD INSERT INTO base_tbl VALUES (NEW.a, NEW.b);
282+
CREATE RULE rw_view16_upd_rule AS ON UPDATE TO rw_view16
283+
WHERE OLD.a > 0 DO INSTEAD UPDATE base_tbl SET b=NEW.b WHERE a=OLD.a;
284+
CREATE RULE rw_view16_del_rule AS ON DELETE TO rw_view16
285+
WHERE OLD.a > 0 DO INSTEAD DELETE FROM base_tbl WHERE a=OLD.a;
286+
INSERT INTO rw_view16 (a, b) VALUES (3, 'Row 3'); -- should fail
287+
ERROR: cannot insert into view "rw_view16"
288+
DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable.
289+
HINT: To enable inserting into the view, provide an INSTEAD OF INSERT trigger or an unconditional ON INSERT DO INSTEAD rule.
290+
UPDATE rw_view16 SET b='ROW 2' WHERE a=2; -- should fail
291+
ERROR: cannot update view "rw_view16"
292+
DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable.
293+
HINT: To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule.
294+
DELETE FROM rw_view16 WHERE a=2; -- should fail
295+
ERROR: cannot delete from view "rw_view16"
296+
DETAIL: Views with conditional DO INSTEAD rules are not automatically updatable.
297+
HINT: To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule.
277298
DROP TABLE base_tbl CASCADE;
278299
NOTICE: drop cascades to 16 other objects
279300
DETAIL: drop cascades to view ro_view1

‎src/test/regress/sql/updatable_views.sql

+14
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,20 @@ DELETE FROM ro_view18;
8989
UPDATE ro_view19 SET max_value=1000;
9090
UPDATE ro_view20 SET b=upper(b);
9191

92+
-- A view with a conditional INSTEAD rule but no unconditional INSTEAD rules
93+
-- or INSTEAD OF triggers should be non-updatable and generate useful error
94+
-- messages with appropriate detail
95+
CREATE RULE rw_view16_ins_rule AS ON INSERT TO rw_view16
96+
WHERE NEW.a > 0 DO INSTEAD INSERT INTO base_tbl VALUES (NEW.a, NEW.b);
97+
CREATE RULE rw_view16_upd_rule AS ON UPDATE TO rw_view16
98+
WHERE OLD.a > 0 DO INSTEAD UPDATE base_tbl SET b=NEW.b WHERE a=OLD.a;
99+
CREATE RULE rw_view16_del_rule AS ON DELETE TO rw_view16
100+
WHERE OLD.a > 0 DO INSTEAD DELETE FROM base_tbl WHERE a=OLD.a;
101+
102+
INSERT INTO rw_view16 (a, b) VALUES (3, 'Row 3'); -- should fail
103+
UPDATE rw_view16 SET b='ROW 2' WHERE a=2; -- should fail
104+
DELETE FROM rw_view16 WHERE a=2; -- should fail
105+
92106
DROP TABLE base_tbl CASCADE;
93107
DROP VIEW ro_view10, ro_view12, ro_view18;
94108
DROP SEQUENCE seq CASCADE;

0 commit comments

Comments
 (0)