Thread: Rules with Conditions: Bug, or Misunderstanding
Am I misunderstanding how to use rule w/conditionals, or is there a bug in this? -- I love to use Pgsql comments, but find the 'comment on field...' language a bit of a pain for documenting a large database at the last minute. So, I wrote a query that pulls together all the fields in a database, w/descriptions (if any): create view dev_col_comments as select a.oid as att_oid, relname, attname, description from pg_class c, pg_attribute a left outer join pg_description d on d.objoid=a.oid where c.oid=a.attrelid and (c.relkind='r' or c.relkind='v') and c.relname !~ '^pg_' and attname not in ('xmax','xmin','cmax','cmin','ctid','oid','tableoid') order by relname, attname; [This uses pg7.1 syntax; you could rewrite for 7.0 w/o the 'v' for views, and using a union rather than outer join.] This works great. Feeling clever, I wrote two rules, so I could update this and create comments. I need two rules, one if this is an existing description (becoming an update to pg_description), one if this not (becoming an insert to pg_description). create rule dev_ins as on update to dev_col_comments where old.description isnull do instead insert into pg_description ( objoid, description) values (old.att_oid, new.description); create rule dev_upd as on update to dev_col_comments where old.description notnull do instead update pg_description set description=new.description where objoid=old.att_oid; This doesn't work: I get a "cannot update view w/o rule" error message, both for fields where description was null, and for fields where it wasn't null. If I take out the "where old.description isnull" clause of dev_ins, it works fine--but, only, of course, if I am sure to only pick new descriptions. Or, if I take out the clause in dev_upd, it works too, with the opposite caveat. Is this a bug? Am I misunderstanding something about the way that rule conditions should work? The docs are long but fuzzy on rules (they seem to suggest, for instance, that "create rule foo on update to table.column" will work, when this is not implemented yet, so perhaps the docs are ahead of the implementation?) Any help would be great! I do read the pgsql lists, but always appreciate a cc, so I don't miss any comments. TIA. Thanks, -- Joel Burton, Director of Information Systems -*- jburton@scw.org Support Center of Washington (www.scw.org)
"Joel Burton" <jburton@scw.org> writes: > create rule dev_ins as on update to dev_col_comments where > old.description isnull do instead insert into pg_description ( objoid, > description) values (old.att_oid, new.description); > create rule dev_upd as on update to dev_col_comments where > old.description notnull do instead update pg_description set > description=new.description where objoid=old.att_oid; > This doesn't work: I get a "cannot update view w/o rule" error > message, both for fields where description was null, and for fields > where it wasn't null. Hm. Perhaps the "cannot update view" test is too strict --- it's not bright enough to realize that the two rules together cover all cases, so it complains that you *might* be trying to update the view. As the code stands, you must provide an unconditional DO INSTEAD rule to implement insertion or update of a view. I'm not sure this is a big problem, though, because the solution is simple: provide an unconditional rule with multiple actions. For example, I think this will work: create rule dev_upd as on update to dev_col_comments do instead ( insert into pg_description (objoid, description) select old.att_oid, new.description WHERE old.description isnull; updatepg_description set description=new.description where objoid = old.att_oid; ) but I haven't tried it... regards, tom lane
On Wednesday 29 November 2000 19:42, Tom Lane wrote: > > Hm. Perhaps the "cannot update view" test is too strict --- it's not > bright enough to realize that the two rules together cover all cases, > so it complains that you *might* be trying to update the view. As the > code stands, you must provide an unconditional DO INSTEAD rule to > implement insertion or update of a view. The idea was to check just before the update occurred to see if the destination was view. Maybe the test is too high up, before all rewriting occurs. It is in InitPlan, the same place we check to make sure that we are not changing a sequence or a toast table. (actually initResultRelInfo called from InitPlan). I gathered from the backend flowchart that this wasn't called until all rewriting was done. Was I wrong? If all rewriting _is_ done at that point, why is the view still in the ResultRelInfo ? -- Mark Hollomon
Mark Hollomon <mhh@mindspring.com> writes: > On Wednesday 29 November 2000 19:42, Tom Lane wrote: >> Hm. Perhaps the "cannot update view" test is too strict --- it's not >> bright enough to realize that the two rules together cover all cases, >> so it complains that you *might* be trying to update the view. As the >> code stands, you must provide an unconditional DO INSTEAD rule to >> implement insertion or update of a view. > It is in InitPlan, the same place we check to make sure that we are > not changing a sequence or a toast table. (actually initResultRelInfo > called from InitPlan). I gathered from the backend flowchart that this > wasn't called until all rewriting was done. Was I wrong? The rewriting is done, all right, but what's left afterward still has references to the view, because each rule is conditional. Essentially, the rewriter output looks like -- rule 1if (rule1 condition holds) -- rule 2 applied to rule1 success case if (rule2 condition holds) applyrule 2's query else apply rule 1's queryelse -- rule 2 applied to rule1 failure case if (rule2 conditionholds) apply rule 2's query else apply original query If the system were capable of determining that either rule1 or rule2 condition will always hold, perhaps it could deduce that the original query on the view will never be applied. However, I doubt that we really want to let loose an automated theorem prover on the results of every rewrite ... regards, tom lane
On 29 Nov 2000, at 19:42, Tom Lane wrote: > "Joel Burton" <jburton@scw.org> writes: > > create rule dev_ins as on update to dev_col_comments where > > old.description isnull do instead insert into pg_description ( > > objoid, description) values (old.att_oid, new.description); > > > create rule dev_upd as on update to dev_col_comments where > > old.description notnull do instead update pg_description set > > description=new.description where objoid=old.att_oid; > > > This doesn't work: I get a "cannot update view w/o rule" error > > message, both for fields where description was null, and for fields > > where it wasn't null. > > [... ] I think this will work: > > create rule dev_upd as on update to dev_col_comments do instead > ( > insert into pg_description (objoid, description) > select old.att_oid, new.description WHERE old.description isnull; > update pg_description set description=new.description > where objoid = old.att_oid; > ) Tom -- Thanks for the help. I had assumed (wrongly) that one could have conditional rules, and only if all the conditions fail, that it would go to the "cannot update view" end, and didn't realize that there *had* to be a single do instead. In any event, though, the rule above crashes my backend, as do simpler versions I wrote that try your CREATE RULE DO INSTEAD ( INSERT; UPDATE; ) idea. What information can I provide to the list to troubleshoot this? Thanks! -- Joel Burton, Director of Information Systems -*- jburton@scw.org Support Center of Washington (www.scw.org)
"Joel Burton" <jburton@scw.org> writes: > In any event, though, the rule above crashes my backend, as do > simpler versions I wrote that try your CREATE RULE DO INSTEAD ( > INSERT; UPDATE; ) idea. Ugh :-( > What information can I provide to the list to troubleshoot this? A gdb backtrace from the corefile, and/or a simple example to replicate the problem... regards, tom lane
On Friday 01 December 2000 00:33, Tom Lane wrote: > The rewriting is done, all right, but what's left afterward still has > references to the view, because each rule is conditional. Essentially, > the rewriter output looks like > > -- rule 1 > if (rule1 condition holds) > -- rule 2 applied to rule1 success case > if (rule2 condition holds) > apply rule 2's query > else > apply rule 1's query > else > -- rule 2 applied to rule1 failure case > if (rule2 condition holds) > apply rule 2's query > else > apply original query > > If the system were capable of determining that either rule1 or rule2 > condition will always hold, perhaps it could deduce that the original > query on the view will never be applied. However, I doubt that we > really want to let loose an automated theorem prover on the results > of every rewrite ... I think it would be better to move the test further down, to just before we actually try to do the update/insert. Maybe into the heap access routines as suggested by Andreas. -- Mark Hollomon
Mark Hollomon <mhh@mindspring.com> writes: > I think it would be better to move the test further down, to just before we > actually try to do the update/insert. Maybe into the heap access routines as > suggested by Andreas. I'm worried about whether it'll be practical to generate a good error message from that low a level. Looking at it from the DBA's viewpoint rather than implementation details, I haven't seen a good reason *why* we should support conditional rules for views, as opposed to an unconditional rule with multiple actions. Seems to me that writing independent rules that you hope will cover all cases is a great way to build an unreliable system. regards, tom lane
Tom Lane wrote: > "Joel Burton" <jburton@scw.org> writes: > > create rule dev_ins as on update to dev_col_comments where > > old.description isnull do instead insert into pg_description ( objoid, > > description) values (old.att_oid, new.description); > > > create rule dev_upd as on update to dev_col_comments where > > old.description notnull do instead update pg_description set > > description=new.description where objoid=old.att_oid; > > > This doesn't work: I get a "cannot update view w/o rule" error > > message, both for fields where description was null, and for fields > > where it wasn't null. > > Hm. Perhaps the "cannot update view" test is too strict --- it's not > bright enough to realize that the two rules together cover all cases, > so it complains that you *might* be trying to update the view. As the > code stands, you must provide an unconditional DO INSTEAD rule to > implement insertion or update of a view. Disagree. A conditional rule splits the command into two, one with the rules action and the condition added, one which is the original statement plus the negated condition. So there are cases left where an INSERT can happen to the view relation and it's the job of this test to prevent it. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck <janwieck@Yahoo.com> writes: > Tom Lane wrote: >> Hm. Perhaps the "cannot update view" test is too strict --- it's not >> bright enough to realize that the two rules together cover all cases, >> so it complains that you *might* be trying to update the view. As the >> code stands, you must provide an unconditional DO INSTEAD rule to >> implement insertion or update of a view. > Disagree. > A conditional rule splits the command into two, one with the > rules action and the condition added, one which is the > original statement plus the negated condition. So there are > cases left where an INSERT can happen to the view relation > and it's the job of this test to prevent it. Well, in that case the present code is broken, because it's going to spit up if any part of the rewritten query shows the view as result relation (cf. QueryRewrite() ... note that this logic no longer looks much like it did the last time you touched it ;-)). You'd have to convert the existing rewrite-time test into a runtime test in order to see whether the query actually tries to insert any tuples into the view. While that is maybe reasonable for insertions, it's totally silly for update and delete queries. Since the view itself can never contain any tuples to be updated or deleted, a runtime test that errors out when one attempts to update or delete such a tuple could never fire. I don't think that means that we shouldn't complain about an update or delete on a view. I think the test is best left as-is... regards, tom lane
Tom Lane wrote: > Jan Wieck <janwieck@Yahoo.com> writes: > > Tom Lane wrote: > >> Hm. Perhaps the "cannot update view" test is too strict --- it's not > >> bright enough to realize that the two rules together cover all cases, > >> so it complains that you *might* be trying to update the view. As the > >> code stands, you must provide an unconditional DO INSTEAD rule to > >> implement insertion or update of a view. > > > Disagree. > > > A conditional rule splits the command into two, one with the > > rules action and the condition added, one which is the > > original statement plus the negated condition. So there are > > cases left where an INSERT can happen to the view relation > > and it's the job of this test to prevent it. > > Well, in that case the present code is broken, because it's going to > spit up if any part of the rewritten query shows the view as result > relation (cf. QueryRewrite() ... note that this logic no longer looks > much like it did the last time you touched it ;-)). You'd have to > convert the existing rewrite-time test into a runtime test in order to > see whether the query actually tries to insert any tuples into the view. Yepp. > While that is maybe reasonable for insertions, it's totally silly > for update and delete queries. Since the view itself can never contain > any tuples to be updated or deleted, a runtime test that errors out > when one attempts to update or delete such a tuple could never fire. > I don't think that means that we shouldn't complain about an update > or delete on a view. > > I think the test is best left as-is... Since conditional rules aren't any better compared to an unconditional multi-action instead rule where the single actions have all the different conditions, let's leave it as is and insist on one unconditional instead rule. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #