Обсуждение: Patch for updatable views
Hi folks, please find attached an implementation for updatable views. Included are support for pg_dump and information_schema, regression test and documentation are missing. Also, there's currently no upgrade path for older PostgreSQL versions and user defined rules on views. I have some code which drops the implicit created rules silently if someone wants to have its own rule, but this needs some discussion, i think. The patch covers the whole SQL92 functionality and doesn't create any rules, if a given view is considered not to be compatible with SQL92 definitions. The supported syntax is CREATE VIEW foo AS .... [ WITH [ CASCADED | LOCAL ] CHECK OPTION ] The check option is implemented as a conditional rule with a simple system function, which checks the given expression tree to be true or false and raises an error in the latter case. There's also a little change in the rewriter semantics, as i treat implicit (view update rules created automatically) and explicit rules (rules created by any user) differently. This involves some changes to the system catalog (especially pg_rewrite and pg_proc), so be prepared to do an initdb. There are new files in src/backend/rewrite/view_update.c and src/include/rewrite/view_update.h, too. Please note that the patch currently breaks some regression tests, but these are mostly due to duplicated rules on views and additional notice messages. Also, i have dropped support for updatable views which contains indexed array fields of tables (like SELECT foo[3], foo[2] FROM bar). These are treated non-updatable and someone needs his own rules here. I hope there aren't too many open points here, so this patch could be considered for inclusion in 8.2. Looking forward your opinions... -- Thanks Bernd
Вложения
On 7/25/06, Bernd Helmle <mailings@oopsware.de> wrote: > Hi folks, > > please find attached an implementation for updatable views. Included are > support for pg_dump and information_schema, regression test and > documentation are missing. Also, there's currently no upgrade path for older > PostgreSQL versions and user defined rules on views. i'm testing the functionality... seems good to me... i will work on docs and regress if no one objects and bernd is not doing it... ------------ AFAICS, the view will not be updateable if there are casts in the select list (seems fair to let that to future revisions), but i think we must say it. ------------ One thing to think of: create table testing_serial (id serial primary key, name text); CREATE TABLE create view vtest_serial as select * from testing_serial; CREATE VIEW insert into vtest_serial values (default, 'name1'); psql:../view_test.sql:81: ERROR: null value in column "id" violates not-null constraint insert into vtest_serial(name) values ('name2'); psql:../view_test.sql:82: ERROR: null value in column "id" violates not-null constraint i still think that in updateable views we need to inherit the defaut value of the base table, i still see this code commented in rewriteHandler.c ------------ psql:../view_test.sql:73: ERROR: cannot insert into a view HINT: You need an unconditional ON INSERT DO INSTEAD rule. BTW, we must change this message for something more like 'cannot insert into a non updateable view' ------------- + /* + * I will do this only in case of relkind == RELKIND_VIEW. + * This is the last attempt to get a value for expr before we + * consider that expr must be NULL. + */ + /* if (expr == NULL && rel->rd_rel->relkind == RELKIND_VIEW) */ + /* { */ + /* expr = (Node *)makeNode(SetToDefault); */ + /* return expr; */ + /* } */ + if this functionality will be accepted this is the time to discuss it otherwise drop this comment. With this code we still can create a different default for the view with ALTER TABLE ADD DEFAULT ------------ > I have some code which drops the implicit created rules silently if someone > wants to have its own rule, but this needs some discussion, i think. > + #if 0 + /* + * Implicit rules should be dropped automatically when someone + * wants to have its *own* rules on the view. is_implicit is set + * to NO_OPTION_EXCPLICIT in this case so we drop all implicit + * rules on the specified event type immediately. + * + * ???FIXME: do we want this behavior??? + */ + + if ( ev_kind == NO_OPTION_EXPLICIT ) + deleteImplicitRulesOnEvent(event_relation, event_type); + #endif This is a must for compatibility with older versions. Otherwise we will have views with user defined rules and implicit rules that will have an unexpected behaviour. ------------ > The patch covers the whole SQL92 functionality and doesn't create any > rules, if a given view is considered not to be compatible with SQL92 definitions. I think is necessary to send some NOTICE when we can't create rules at all or when we can't create one of them (insert rules are not always created because they need all not-null without defaults columns to be in the select list) ------------ > The supported > syntax is > > CREATE VIEW foo AS .... [ WITH [ CASCADED | LOCAL ] CHECK OPTION ] > > The check option is implemented as a conditional rule with a simple system > function, which checks the given expression tree to be true or false and raises > an error in the latter case. the check option is working for all cases i'm trying... > Also, i have dropped support for updatable views which contains indexed array > fields of tables (like SELECT foo[3], foo[2] FROM bar). These are treated > non-updatable and someone needs his own rules here. > -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
--On Dienstag, Juli 25, 2006 18:29:39 -0500 Jaime Casanova <systemguards@gmail.com> wrote: > On 7/25/06, Bernd Helmle <mailings@oopsware.de> wrote: >> Hi folks, >> >> please find attached an implementation for updatable views. Included are >> support for pg_dump and information_schema, regression test and >> documentation are missing. Also, there's currently no upgrade path for >> older PostgreSQL versions and user defined rules on views. > > i'm testing the functionality... seems good to me... i will work on > docs and regress if no one objects and bernd is not doing it... > > ------------ > AFAICS, the view will not be updateable if there are casts in the > select list (seems fair to let that to future revisions), but i think > we must say it. > I thought about it a while and came to the conclusion that this makes no sense. Casting in a view's target list involves computation, function calls or constant values usually, so i think there's no reason to add more complexity to the code. > ------------ > One thing to think of: > > create table testing_serial (id serial primary key, name text); > CREATE TABLE > > create view vtest_serial as select * from testing_serial; > CREATE VIEW > > insert into vtest_serial values (default, 'name1'); > psql:../view_test.sql:81: ERROR: null value in column "id" violates > not-null constraint > > insert into vtest_serial(name) values ('name2'); > psql:../view_test.sql:82: ERROR: null value in column "id" violates > not-null constraint > > i still think that in updateable views we need to inherit the defaut > value of the base table, i still see this code commented in > rewriteHandler.c > Err, forgot to change the comment into a #if..#end block... It's easy to add column defaults to a view if you need them, but i'm not sure creating them automatically is the right way. > ------------ > psql:../view_test.sql:73: ERROR: cannot insert into a view > HINT: You need an unconditional ON INSERT DO INSTEAD rule. > > BTW, we must change this message for something more like 'cannot > insert into a non updateable view' > > ------------- > + /* > + * I will do this only in case of relkind == RELKIND_VIEW. > + * This is the last attempt to get a value for expr before we > + * consider that expr must be NULL. > + */ > + /* if (expr == NULL && rel->rd_rel->relkind == RELKIND_VIEW) */ > + /* { */ > + /* expr = (Node *)makeNode(SetToDefault); */ > + /* return expr; */ > + /* } */ > + > > if this functionality will be accepted this is the time to discuss it > otherwise drop this comment. > Yepp. > With this code we still can create a different default for the view > with ALTER TABLE ADD DEFAULT > Hmm, this is something i haven't considered yet.... > ------------ >> I have some code which drops the implicit created rules silently if >> someone wants to have its own rule, but this needs some discussion, i >> think. >> > > + #if 0 > + /* > + * Implicit rules should be dropped automatically when someone > + * wants to have its *own* rules on the view. is_implicit is set > + * to NO_OPTION_EXCPLICIT in this case so we drop all implicit > + * rules on the specified event type immediately. > + * > + * ???FIXME: do we want this behavior??? > + */ > + > + if ( ev_kind == NO_OPTION_EXPLICIT ) > + deleteImplicitRulesOnEvent(event_relation, event_type); > + #endif > > This is a must for compatibility with older versions. Otherwise we > will have views with user defined rules and implicit rules that will > have an unexpected behaviour. > Like the rule regression tests which fail exactly because of this reason. However, i'm not sure if the backend is the right place to do such implicit things. One idea is to provide a stored procedure that drops implicit or user defined rules on views (this could be implemented easily) and to instruct users to follow this upgrade path (or to teach pg_dump to do so....). > ------------ >> The patch covers the whole SQL92 functionality and doesn't create any >> rules, if a given view is considered not to be compatible with SQL92 >> definitions. > > I think is necessary to send some NOTICE when we can't create rules at > all or when we can't create one of them (insert rules are not always > created because they need all not-null without defaults columns to be > in the select list) > I think we could do it the other way, like the NOTICEs you get when you create tables with primary keys. This would be more consistent then. [...] -- Thanks Bernd