Обсуждение: Allow WHEN in INSTEAD OF triggers

Folks,

While noodling around with an upcoming patch to remove user-modifiable
RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
triggers for no discernible reason. This patch removes that
restriction.

I noticed that columns were also disallowed in INSTEAD OF triggers,
but haven't dug further into those just yet.

What say?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On 2019-Dec-28, David Fetter wrote:

> While noodling around with an upcoming patch to remove user-modifiable
> RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
> triggers for no discernible reason. This patch removes that
> restriction.

If you want to remove the restriction, your patch should add some test
cases that show it working.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

David Fetter <david@fetter.org> writes:
> While noodling around with an upcoming patch to remove user-modifiable
> RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
> triggers for no discernible reason. This patch removes that
> restriction.

This seems like a remarkably bad idea.  The point of an INSTEAD OF
trigger is that it is guaranteed to handle the operation.  What's
the system supposed to do with rows the trigger doesn't handle?

I notice that your patch doesn't even bother to test what happens,
but I'd argue that whatever it is, it's wrong.  If you think that
"do nothing" or "throw an error" is appropriate, you can code that
inside the trigger.  It's not PG's charter to make such a decision.

            regards, tom lane

PS: I think your chances of removing rules are not good, either.

On Fri, Dec 27, 2019 at 10:29:15PM -0500, Tom Lane wrote:
> David Fetter <david@fetter.org> writes:
> > While noodling around with an upcoming patch to remove user-modifiable
> > RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
> > triggers for no discernible reason. This patch removes that
> > restriction.
> 
> This seems like a remarkably bad idea.  The point of an INSTEAD OF
> trigger is that it is guaranteed to handle the operation.  What's
> the system supposed to do with rows the trigger doesn't handle?

Nothing.  Why would it be different from the other forms of WHEN in
triggers?

> I notice that your patch doesn't even bother to test what happens,
> but I'd argue that whatever it is, it's wrong.  If you think that
> "do nothing" or "throw an error" is appropriate, you can code that
> inside the trigger.  It's not PG's charter to make such a decision.

If that's the case, why do we have WHEN for triggers at all? I'm just
working toward make them more consistent. From a UX perspective, it's
a lot simpler and clearer to do this in the trigger declaration than
it is in the body.

> PS: I think your chances of removing rules are not good, either.

I suspect I have a lot of company in my view of user-modifiable
rewrite rules as an experiment we can finally discontinue in view of
its decisive results.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Sat, Dec 28, 2019 at 12:12:30AM -0300, Alvaro Herrera wrote:
> On 2019-Dec-28, David Fetter wrote:
> 
> > While noodling around with an upcoming patch to remove user-modifiable
> > RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
> > triggers for no discernible reason. This patch removes that
> > restriction.
> 
> If you want to remove the restriction, your patch should add some test
> cases that show it working.

Tests added :)

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Sat, 28 Dec 2019 at 16:45, David Fetter <david@fetter.org> wrote:
>
> On Sat, Dec 28, 2019 at 12:12:30AM -0300, Alvaro Herrera wrote:
> > On 2019-Dec-28, David Fetter wrote:
> >
> > > While noodling around with an upcoming patch to remove user-modifiable
> > > RULEs, I noticed that WHEN conditions were disallowed from INSTEAD OF
> > > triggers for no discernible reason. This patch removes that
> > > restriction.
> >
> > If you want to remove the restriction, your patch should add some test
> > cases that show it working.
>
> Tests added :)
>

I too think this is a bad idea.

Doing nothing if the trigger's WHEN condition isn't satisfied is not
consistent with the way other types of trigger work -- with any other
type of trigger, if the WHEN condition doesn't evaluate to true, the
query goes ahead as if the trigger hadn't been there. So the most
consistent thing to do would be to attempt an auto-update if the
trigger isn't fired, and that leads to a whole other world of pain
(e.g., you'd need 2 completely different query plans for the 2 cases,
and more if you had views on top of other views).

The SQL spec explicitly states that INSTEAD OF triggers on views
should not have WHEN clauses, and for good reason. There are cases
where it makes sense to deviate from the spec, but I don't think this
is one of them.

Regards,
Dean