Re: Proof of concept: auto updatable views [Review of Patch]

Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> writes:
> On 7 November 2012 22:04, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This seems to me to be dangerous and unintuitive, not to mention
>> underdocumented. I think it would be better to just not do anything if
>> there is any INSTEAD rule, period.

> Is this any more dangerous than what already happens with qualified rules?

> If we did nothing here then it would go on to either fire any INSTEAD
> OF triggers or raise an error if there aren't any. The problem with
> that is that it makes trigger-updatable views and auto-updatable views
> inconsistent in their behaviour with qualified INSTEAD rules.

Well, as submitted it's already pretty thoroughly inconsistent. The way
the existing code works is that if there's no INSTEAD rule, and there's
no INSTEAD trigger, you get an error *at runtime*. The reason for that
is that the INSTEAD trigger might be added (or removed) between planning
and execution. This code tries to decide at plan time whether there's a
relevant trigger, and that's just not very safe.

I realize that you can't deliver the specific error messages that
currently appear in view_is_auto_updatable if you don't throw the error
at plan time. But if you're going to claim that this ought to be
consistent with the existing behavior, then I'm going to say we need to
give that up and just have the runtime error, same as now.

If you want the better error reporting (which I agree would be nice)
then we need to revisit the interaction between INSTEAD triggers and
INSTEAD rules anyway, and one of the things we probably should look at
twice is whether it's sane at all to permit both a trigger and a
qualified rule. I'd bet long odds that nobody is using such a thing in
the field, and I think disallowing it might be a good idea in order to
disentangle these features a bit better.

regards, tom lane