Обсуждение: Tables cannot have INSTEAD OF triggers

On Tue, Mar 31, 2015 at 8:49 AM, Aliouii Ali <aliouii.ali@aol.fr> wrote:
> hi all,
> back in
> 2011(http://www.postgresql.org/message-id/1305138588.8811.3.camel@vanquo.pezone.net),
> an question the same as this one was asked
> the anwser was :
>
> I think they're very useful on views, but I
> couldn't think of a use-case for having them on tables. ISTM that
> anything an INSTEAD OF trigger on a table could do, could equally well
> be done in a BEFORE trigger.
> no not really there is a use-case : in partitioned table ( instead of
> defining before trigger on the master table that return null as the doc
> states, it will be good things to have instead of trigger that return NEW)
> so that query like insert/update ... .. RETURNING will be handdy and gain
> some performance, otherwise we will have to do an insert and select to get
> the same jobs done

I don't see how this helps.  The problem with partitioning is that you
need a way to redirect the INSERT to another table, and there's no
built-in way to do that, so you have to simulate it somehow.  That
issue seems largely separate from how the CREATE TRIGGER command is
spelled.  Maybe I'm missing something.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2015-04-01 11:40:13 -0400, Robert Haas wrote:
> On Tue, Mar 31, 2015 at 8:49 AM, Aliouii Ali <aliouii.ali@aol.fr> wrote:
> I don't see how this helps.  The problem with partitioning is that you
> need a way to redirect the INSERT to another table, and there's no
> built-in way to do that, so you have to simulate it somehow.  That
> issue seems largely separate from how the CREATE TRIGGER command is
> spelled.  Maybe I'm missing something.

Without INSTEAD OF you can't, to my knowledge, return a valid tuple from
the top level table without also inserting into it. Returning NULL after
redirecting the tuple into a child table will break RETURNING; not
returning NULL will insert the tuple in the top level table.

So the only way to do redirection that doesn't break RETURNING without
rules is to insert the tuple in the child in the BEFORE trigger return
NEW and delete the top level table row in an AFTER trigger. That sucks.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On Wed, Apr 1, 2015 at 12:04 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2015-04-01 11:40:13 -0400, Robert Haas wrote:
>> I don't see how this helps.  The problem with partitioning is that you
>> need a way to redirect the INSERT to another table, and there's no
>> built-in way to do that, so you have to simulate it somehow.  That
>> issue seems largely separate from how the CREATE TRIGGER command is
>> spelled.  Maybe I'm missing something.
>
> Without INSTEAD OF you can't, to my knowledge, return a valid tuple from
> the top level table without also inserting into it. Returning NULL after
> redirecting the tuple into a child table will break RETURNING; not
> returning NULL will insert the tuple in the top level table.
>
> So the only way to do redirection that doesn't break RETURNING without
> rules is to insert the tuple in the child in the BEFORE trigger return
> NEW and delete the top level table row in an AFTER trigger. That sucks.

So, the idea is that INSTEAD OF would behave like BEFORE but the tuple
it returns wouldn't actually be inserted?  That wasn't clear to me
from the OP, but I guess it would be a reasonable way to go.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2015-04-01 12:46:05 -0400, Robert Haas wrote:
> On Wed, Apr 1, 2015 at 12:04 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> > On 2015-04-01 11:40:13 -0400, Robert Haas wrote:
> > Without INSTEAD OF you can't, to my knowledge, return a valid tuple from
> > the top level table without also inserting into it. Returning NULL after
> > redirecting the tuple into a child table will break RETURNING; not
> > returning NULL will insert the tuple in the top level table.
> >
> > So the only way to do redirection that doesn't break RETURNING without
> > rules is to insert the tuple in the child in the BEFORE trigger return
> > NEW and delete the top level table row in an AFTER trigger. That sucks.
> 
> So, the idea is that INSTEAD OF would behave like BEFORE but the tuple
> it returns wouldn't actually be inserted?  That wasn't clear to me
> from the OP, but I guess it would be a reasonable way to go.

I'm not sure what the OP intended, but to me that's pretty much the only
reasonable definition of INSTEAD OF for tables that I can think of.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2015-04-01 12:46:05 -0400, Robert Haas wrote:
>> So, the idea is that INSTEAD OF would behave like BEFORE but the tuple
>> it returns wouldn't actually be inserted?  That wasn't clear to me
>> from the OP, but I guess it would be a reasonable way to go.

> I'm not sure what the OP intended, but to me that's pretty much the only
> reasonable definition of INSTEAD OF for tables that I can think of.

If you have such a trigger, it's impossible to insert any rows, which
means the table doesn't need storage, which means it may as well be a
view, no?  So this still seems to me like a wart not a useful feature.
I think it would create confusion because a table with such a trigger
would act so much unlike other tables.
        regards, tom lane

On 2015-04-01 13:15:26 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2015-04-01 12:46:05 -0400, Robert Haas wrote:
> >> So, the idea is that INSTEAD OF would behave like BEFORE but the tuple
> >> it returns wouldn't actually be inserted?  That wasn't clear to me
> >> from the OP, but I guess it would be a reasonable way to go.
> 
> > I'm not sure what the OP intended, but to me that's pretty much the only
> > reasonable definition of INSTEAD OF for tables that I can think of.
> 
> If you have such a trigger, it's impossible to insert any rows, which
> means the table doesn't need storage, which means it may as well be a
> view, no?  So this still seems to me like a wart not a useful feature.
> I think it would create confusion because a table with such a trigger
> would act so much unlike other tables.

For one you can't easily add partitions to a view (and
constraint_exclusion = partition IIRC doesn't work if you use UNION ALL),
for another there's WHEN for triggers that should allow dealing with
that.

Greetings,

Andres Freund

Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2015-04-01 12:46:05 -0400, Robert Haas wrote:
> >> So, the idea is that INSTEAD OF would behave like BEFORE but the tuple
> >> it returns wouldn't actually be inserted?  That wasn't clear to me
> >> from the OP, but I guess it would be a reasonable way to go.
> 
> > I'm not sure what the OP intended, but to me that's pretty much the only
> > reasonable definition of INSTEAD OF for tables that I can think of.
> 
> If you have such a trigger, it's impossible to insert any rows, which
> means the table doesn't need storage, which means it may as well be a
> view, no?

The interesting difference, as per upthread, is that you can have child
tables (partitions) and don't need a defining query but instead have a
defined set of named and typed columns.

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

Andres Freund <andres@anarazel.de> writes:
> On 2015-04-01 13:15:26 -0400, Tom Lane wrote:
>> If you have such a trigger, it's impossible to insert any rows, which
>> means the table doesn't need storage, which means it may as well be a
>> view, no?  So this still seems to me like a wart not a useful feature.
>> I think it would create confusion because a table with such a trigger
>> would act so much unlike other tables.

> For one you can't easily add partitions to a view (and
> constraint_exclusion = partition IIRC doesn't work if you use UNION ALL),
> for another there's WHEN for triggers that should allow dealing with
> that.

WHEN won't help; if there are any INSTEAD OF triggers, no insert will
happen, whether the triggers actually fire or not.

As for partitioning, you could do this:

create table parent(...);
create table child(...) inherits(parent); -- repeat as needed
create view v as select * from parent;
attach INSTEAD OF triggers to v

Now the application deals only with v, and thinks that's the real
table.
        regards, tom lane

On 2015-04-01 13:29:33 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2015-04-01 13:15:26 -0400, Tom Lane wrote:
> >> If you have such a trigger, it's impossible to insert any rows, which
> >> means the table doesn't need storage, which means it may as well be a
> >> view, no?  So this still seems to me like a wart not a useful feature.
> >> I think it would create confusion because a table with such a trigger
> >> would act so much unlike other tables.
> 
> > For one you can't easily add partitions to a view (and
> > constraint_exclusion = partition IIRC doesn't work if you use UNION ALL),
> > for another there's WHEN for triggers that should allow dealing with
> > that.
> 
> WHEN won't help; if there are any INSTEAD OF triggers, no insert will
> happen, whether the triggers actually fire or not.

Well, right now it doesn't work at all. It seems pretty reasonable to
define things so that the insert happens normally if there's no matching
INSTEAD OF trigger.

> As for partitioning, you could do this:
> 
> create table parent(...);
> create table child(...) inherits(parent); -- repeat as needed
> create view v as select * from parent;
> attach INSTEAD OF triggers to v
> 
> Now the application deals only with v, and thinks that's the real
> table.

Sure, but that's just making things unnecessarily hard. That then
requires also defining UPDATE/DELETE INSTEAD triggers which otherwise
would just work.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2015-04-01 13:29:33 -0400, Tom Lane wrote:
>> WHEN won't help; if there are any INSTEAD OF triggers, no insert will
>> happen, whether the triggers actually fire or not.

> Well, right now it doesn't work at all. It seems pretty reasonable to
> define things so that the insert happens normally if there's no matching
> INSTEAD OF trigger.

It would absolutely *not* be reasonable for WHEN conditions for triggers
on tables to work completely differently than they do for triggers on
views.  That ship's sailed.
        regards, tom lane

On 1 April 2015 at 18:37, Andres Freund <andres@anarazel.de> wrote:
> On 2015-04-01 13:29:33 -0400, Tom Lane wrote:
>> As for partitioning, you could do this:
>>
>> create table parent(...);
>> create table child(...) inherits(parent); -- repeat as needed
>> create view v as select * from parent;
>> attach INSTEAD OF triggers to v
>>
>> Now the application deals only with v, and thinks that's the real
>> table.
>
> Sure, but that's just making things unnecessarily hard. That then
> requires also defining UPDATE/DELETE INSTEAD triggers which otherwise
> would just work.
>

No, because as defined above the view v would be auto-updatable, so
updates and deletes on v would just do the matching update/delete on
parent.

Regards,
Dean

On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2015-04-01 13:29:33 -0400, Tom Lane wrote:
>>> WHEN won't help; if there are any INSTEAD OF triggers, no insert will
>>> happen, whether the triggers actually fire or not.
>
>> Well, right now it doesn't work at all. It seems pretty reasonable to
>> define things so that the insert happens normally if there's no matching
>> INSTEAD OF trigger.
>
> It would absolutely *not* be reasonable for WHEN conditions for triggers
> on tables to work completely differently than they do for triggers on
> views.  That ship's sailed.

Clue me in, because I'm confused.  If no trigger fires, we do whatever
an object of that type would normally do in the absence of any
trigger, no?  For a view, that's error out; for a table, that's
perform the action on the underlying data.  That doesn't seem terribly
unprincipled.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It would absolutely *not* be reasonable for WHEN conditions for triggers
>> on tables to work completely differently than they do for triggers on
>> views.  That ship's sailed.

> Clue me in, because I'm confused.  If no trigger fires, we do whatever
> an object of that type would normally do in the absence of any
> trigger, no?  For a view, that's error out; for a table, that's
> perform the action on the underlying data.  That doesn't seem terribly
> unprincipled.

I dunno about unprincipled; but we have already laid down the definition
of INSTEAD OF triggers, and they act as I described.  Read the code if you
doubt it: which path is taken in ExecInsert depends only on whether
INSTEAD OF triggers *exist* on the rel, not whether any of them actually
fired (indeed, it would be difficult even to know that from here).
I believe this was intentional, not just a coding artifact; it stems from
having wanted to throw the error for uninsertable view well upstream of
here, rather than having it be conditional on what happens at runtime.

What I am objecting to is Andres' claim that it would be okay for INSTEAD
OF triggers on tables to act completely differently in this regard from
those on views.  We have laid down the definition for views, and it is
that nothing happens if the trigger exists but doesn't fire.
        regards, tom lane

On 2 April 2015 at 14:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> It would absolutely *not* be reasonable for WHEN conditions for triggers
>>> on tables to work completely differently than they do for triggers on
>>> views.  That ship's sailed.
>
>> Clue me in, because I'm confused.  If no trigger fires, we do whatever
>> an object of that type would normally do in the absence of any
>> trigger, no?  For a view, that's error out; for a table, that's
>> perform the action on the underlying data.  That doesn't seem terribly
>> unprincipled.
>
> I dunno about unprincipled; but we have already laid down the definition
> of INSTEAD OF triggers, and they act as I described.  Read the code if you
> doubt it: which path is taken in ExecInsert depends only on whether
> INSTEAD OF triggers *exist* on the rel, not whether any of them actually
> fired (indeed, it would be difficult even to know that from here).
> I believe this was intentional, not just a coding artifact; it stems from
> having wanted to throw the error for uninsertable view well upstream of
> here, rather than having it be conditional on what happens at runtime.
>
> What I am objecting to is Andres' claim that it would be okay for INSTEAD
> OF triggers on tables to act completely differently in this regard from
> those on views.  We have laid down the definition for views, and it is
> that nothing happens if the trigger exists but doesn't fire.
>

Well actually the fact that the code is structured that way is
somewhat academic. INSTEAD OF triggers on views don't support WHEN
conditions -- deliberately so, since it would be difficult to know in
general what to do if the trigger didn't fire. So ExecInsert is
implicitly using the existence of the trigger to imply that it will
fire, although arguably it would be neater for it to double-check
that, and error out if for some reason the trigger didn't fire. In any
case, that doesn't establish any kind of behavioural precedent for how
a conditional INSTEAD OF trigger on a table ought to work.

Regards,
Dean

On 4/2/15 11:50 AM, Dean Rasheed wrote:
> Well actually the fact that the code is structured that way is
> somewhat academic. INSTEAD OF triggers on views don't support WHEN
> conditions -- deliberately so, since it would be difficult to know in
> general what to do if the trigger didn't fire. So ExecInsert is
> implicitly using the existence of the trigger to imply that it will
> fire, although arguably it would be neater for it to double-check
> that, and error out if for some reason the trigger didn't fire. In any
> case, that doesn't establish any kind of behavioural precedent for how
> a conditional INSTEAD OF trigger on a table ought to work.

I think the upshot is that INSTEAD OF triggers work in a particular way
because that's what is needed to support updatable views.  If triggers
on tables should behave differently, maybe it should be a separate
trigger type.  Maybe it would be feasible to extend BEFORE triggers to
support RETURNING, for example?

On 2015-04-02 16:42:43 -0400, Peter Eisentraut wrote:
> On 4/2/15 11:50 AM, Dean Rasheed wrote:
> > Well actually the fact that the code is structured that way is
> > somewhat academic. INSTEAD OF triggers on views don't support WHEN
> > conditions -- deliberately so, since it would be difficult to know in
> > general what to do if the trigger didn't fire. So ExecInsert is
> > implicitly using the existence of the trigger to imply that it will
> > fire, although arguably it would be neater for it to double-check
> > that, and error out if for some reason the trigger didn't fire. In any
> > case, that doesn't establish any kind of behavioural precedent for how
> > a conditional INSTEAD OF trigger on a table ought to work.
> 
> I think the upshot is that INSTEAD OF triggers work in a particular way
> because that's what is needed to support updatable views.  If triggers
> on tables should behave differently, maybe it should be a separate
> trigger type.  Maybe it would be feasible to extend BEFORE triggers to
> support RETURNING, for example?

What in the above prohibits extending the behaviour to tables? I have
yet to see what compatibility or similarity problem that'd pose. It
seems all mightily handwavy to me.

Greetings,

Andres Freund

On Thu, Apr 2, 2015 at 5:02 PM, Andres Freund <andres@anarazel.de> wrote:
>> I think the upshot is that INSTEAD OF triggers work in a particular way
>> because that's what is needed to support updatable views.  If triggers
>> on tables should behave differently, maybe it should be a separate
>> trigger type.  Maybe it would be feasible to extend BEFORE triggers to
>> support RETURNING, for example?
>
> What in the above prohibits extending the behaviour to tables? I have
> yet to see what compatibility or similarity problem that'd pose. It
> seems all mightily handwavy to me.

Yeah.  It's possible there's a better interface here than INSTEAD OF,
and one of the things I didn't like about the OP was that it started
by stating the syntax that would be used rather than by describing the
problem that needed to be solved.  It's generally better to start with
the latter, and then work out the syntax from there.  But having
gotten that gripe out of my system, and especially in view of Dean's
comments, it's not very clear to me what's wrong with using INSTEAD OF
for this purpose.  If you make BEFORE triggers do this via RETURNING,
then you might have a trigger that returns multiple rows, which seems
like it would introduce a bunch of new complexity for no obvious
benefit.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2 April 2015 at 22:23, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Apr 2, 2015 at 5:02 PM, Andres Freund <andres@anarazel.de> wrote:
>>> I think the upshot is that INSTEAD OF triggers work in a particular way
>>> because that's what is needed to support updatable views.  If triggers
>>> on tables should behave differently, maybe it should be a separate
>>> trigger type.  Maybe it would be feasible to extend BEFORE triggers to
>>> support RETURNING, for example?
>>
>> What in the above prohibits extending the behaviour to tables? I have
>> yet to see what compatibility or similarity problem that'd pose. It
>> seems all mightily handwavy to me.
>
> Yeah.  It's possible there's a better interface here than INSTEAD OF,
> and one of the things I didn't like about the OP was that it started
> by stating the syntax that would be used rather than by describing the
> problem that needed to be solved.  It's generally better to start with
> the latter, and then work out the syntax from there.  But having
> gotten that gripe out of my system, and especially in view of Dean's
> comments, it's not very clear to me what's wrong with using INSTEAD OF
> for this purpose.  If you make BEFORE triggers do this via RETURNING,
> then you might have a trigger that returns multiple rows, which seems
> like it would introduce a bunch of new complexity for no obvious
> benefit.
>

Yes, I'm inclined to agree. One of the reasons that INSTEAD OF
triggers weren't supported on tables was the lack of an obvious
use-case for it, but now having thought about partitioning, I think
they would provide a fairly neat solution to that problem. I don't
think that putting a view with INSTEAD OF triggers on top of the
parent table and then always going through that view works quite as
well, because there are still a few cases where a view doesn't work as
well as a table. A view can't be the target of a foreign key, for
example, so there'd be no way for a cascaded UPDATE to invoke the
INSTEAD OF triggers.

If you needed to handle the case of updates causing a change of
partition, adding conditional INSTEAD OF triggers to the child tables
would be a way to do that, retaining support for RETURNING, and
keeping the logic localised to each partition, only invoking it when
necessary.

Supporting INSTEAD OF triggers on tables is not completely trivial to
implement, but it doesn't look too hard either, and the more I think
about it, the more I suspect that other use-cases will emerge to make
that effort worthwhile.

Regards,
Dean