Thread: 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
Attachment
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
Attachment
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