Thread: [DOCS] Confusing Trigger Docs.

[DOCS] Confusing Trigger Docs.

From
neil@fairwindsoft.com
Date:
The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
Description:

https://www.postgresql.org/docs/devel/static/trigger-definition.html

This sentence:

"If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
UPDATE triggers can both be applied in a way that is apparent from the final
state of the updated row, if an EXCLUDED column is referenced."

is very hard to digest.

Should "is apparent" really be "is not apparent"?  If not, what does "is
apparent" mean and why is this statement here?

Then

"There need not be an EXCLUDED column reference for both sets of row-level
BEFORE triggers to execute, though."

Does this mean that both row level BEFORE INSERT and BEFORE UPDATE triggers
are always executed when ON CONFLICT DO UPDATE clause is present?  Or is
there some circumstance where they are not?

If so, does this also mean that if I have a single trigger defined as BEFORE
UPDATE OR INSERT that this trigger will fire twice?

Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
Can someone help with this text?  I think the author is right that it
needs correction or clarification.

---------------------------------------------------------------------------

On Mon, Jul  3, 2017 at 08:07:10PM +0000, neil@fairwindsoft.com wrote:
> The following documentation comment has been logged on the website:
>
> Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
> Description:
>
> https://www.postgresql.org/docs/devel/static/trigger-definition.html
>
> This sentence:
>
> "If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
> the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
> UPDATE triggers can both be applied in a way that is apparent from the final
> state of the updated row, if an EXCLUDED column is referenced."
>
> is very hard to digest.
>
> Should "is apparent" really be "is not apparent"?  If not, what does "is
> apparent" mean and why is this statement here?
>
> Then
>
> "There need not be an EXCLUDED column reference for both sets of row-level
> BEFORE triggers to execute, though."
>
> Does this mean that both row level BEFORE INSERT and BEFORE UPDATE triggers
> are always executed when ON CONFLICT DO UPDATE clause is present?  Or is
> there some circumstance where they are not?
>
> If so, does this also mean that if I have a single trigger defined as BEFORE
> UPDATE OR INSERT that this trigger will fire twice?
>
> --
> Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-docs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +


Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
[ This time with the right CCs.]

Can someone help with this text?  I think the author is right that it
needs correction or clarification.

---------------------------------------------------------------------------

On Mon, Jul  3, 2017 at 08:07:10PM +0000, neil@fairwindsoft.com wrote:
> The following documentation comment has been logged on the website:
>
> Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
> Description:
>
> https://www.postgresql.org/docs/devel/static/trigger-definition.html
>
> This sentence:
>
> "If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
> the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
> UPDATE triggers can both be applied in a way that is apparent from the final
> state of the updated row, if an EXCLUDED column is referenced."
>
> is very hard to digest.
>
> Should "is apparent" really be "is not apparent"?  If not, what does "is
> apparent" mean and why is this statement here?
>
> Then
>
> "There need not be an EXCLUDED column reference for both sets of row-level
> BEFORE triggers to execute, though."
>
> Does this mean that both row level BEFORE INSERT and BEFORE UPDATE triggers
> are always executed when ON CONFLICT DO UPDATE clause is present?  Or is
> there some circumstance where they are not?
>
> If so, does this also mean that if I have a single trigger defined as BEFORE
> UPDATE OR INSERT that this trigger will fire twice?
>
> --
> Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-docs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +


Re: [DOCS] Confusing Trigger Docs.

From
Peter Geoghegan
Date:
On Thu, Aug 31, 2017 at 6:25 AM, Bruce Momjian <bruce@momjian.us> wrote:
> On Mon, Jul  3, 2017 at 08:07:10PM +0000, neil@fairwindsoft.com wrote:
>> The following documentation comment has been logged on the website:
>>
>> Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
>> Description:
>>
>> https://www.postgresql.org/docs/devel/static/trigger-definition.html
>>
>> This sentence:
>>
>> "If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
>> the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
>> UPDATE triggers can both be applied in a way that is apparent from the final
>> state of the updated row, if an EXCLUDED column is referenced."
>>
>> is very hard to digest.

EXCLUDED.* is exactly what the name suggests -- the tuple that was not
inserted because of a conflict. So, naturally it has the effects of
any before insert trigger, and carries them forward. But you still
have before triggers on the update side.

Typically, this won't matter at all, because before triggers tend to
be written in an idempotent fashion -- something gets filled in. But I
can imagine cases where it is not idempotent, and apply a before
update trigger modifies the row in a way that is surprising. Just
because ON CONFLICT DO UPDATE was used rather than UPDATE. That's what
the documentation warns about.

--
Peter Geoghegan


Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
On Thu, Aug 31, 2017 at 09:22:22AM -0700, Peter Geoghegan wrote:
> On Thu, Aug 31, 2017 at 6:25 AM, Bruce Momjian <bruce@momjian.us> wrote:
> > On Mon, Jul  3, 2017 at 08:07:10PM +0000, neil@fairwindsoft.com wrote:
> >> The following documentation comment has been logged on the website:
> >>
> >> Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
> >> Description:
> >>
> >> https://www.postgresql.org/docs/devel/static/trigger-definition.html
> >>
> >> This sentence:
> >>
> >> "If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
> >> the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
> >> UPDATE triggers can both be applied in a way that is apparent from the final
> >> state of the updated row, if an EXCLUDED column is referenced."
> >>
> >> is very hard to digest.
> 
> EXCLUDED.* is exactly what the name suggests -- the tuple that was not
> inserted because of a conflict. So, naturally it has the effects of
> any before insert trigger, and carries them forward. But you still
> have before triggers on the update side.
> 
> Typically, this won't matter at all, because before triggers tend to
> be written in an idempotent fashion -- something gets filled in. But I
> can imagine cases where it is not idempotent, and apply a before
> update trigger modifies the row in a way that is surprising. Just
> because ON CONFLICT DO UPDATE was used rather than UPDATE. That's what
> the documentation warns about.

I know this thread is six years old, but I still found it confusing, so
the attached patch tries to simplify it.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: [DOCS] Confusing Trigger Docs.

From
Laurenz Albe
Date:
On Tue, 2023-11-21 at 21:01 -0500, Bruce Momjian wrote:
> On Thu, Aug 31, 2017 at 09:22:22AM -0700, Peter Geoghegan wrote:
> > On Thu, Aug 31, 2017 at 6:25 AM, Bruce Momjian <bruce@momjian.us> wrote:
> > > On Mon, Jul  3, 2017 at 08:07:10PM +0000, neil@fairwindsoft.com wrote:
> > > > The following documentation comment has been logged on the website:
> > > >
> > > > Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
> > > > Description:
> > > >
> > > > https://www.postgresql.org/docs/devel/static/trigger-definition.html
> > > >
> > > > This sentence:
> > > >
> > > > "If an INSERT contains an ON CONFLICT DO UPDATE clause, it is possible that
> > > > the effects of all row-level BEFORE INSERT triggers and all row-level BEFORE
> > > > UPDATE triggers can both be applied in a way that is apparent from the final
> > > > state of the updated row, if an EXCLUDED column is referenced."
> > > >
> > > > is very hard to digest.
> >
> > EXCLUDED.* is exactly what the name suggests -- the tuple that was not
> > inserted because of a conflict. So, naturally it has the effects of
> > any before insert trigger, and carries them forward. But you still
> > have before triggers on the update side.
> >
> > Typically, this won't matter at all, because before triggers tend to
> > be written in an idempotent fashion -- something gets filled in. But I
> > can imagine cases where it is not idempotent, and apply a before
> > update trigger modifies the row in a way that is surprising. Just
> > because ON CONFLICT DO UPDATE was used rather than UPDATE. That's what
> > the documentation warns about.
>
> I know this thread is six years old, but I still found it confusing, so
> the attached patch tries to simplify it.

I agree that the paragraph you are trying to improve needs it.

I am not sure about that last sentence you added:

  The modification of
  <varname>EXCLUDED</varname> columns has similar interactions.

How do you modify an EXCLUDED column?  Are you talking about a BEFORE
INSERT trigger?  Reading the original text, I get the impression that
it means "the behavior is obvious if you modify a column that is used
with EXCLUDED in the DO UPDATE clause, but it can also happen if that
column is not user with EXCLUDED".

Perhaps you should omit that sentence for clarity.

Yours,
Laurenz Albe



Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
On Wed, Nov 22, 2023 at 10:31:25AM +0100, Laurenz Albe wrote:
> I agree that the paragraph you are trying to improve needs it.
> 
> I am not sure about that last sentence you added:
> 
>   The modification of
>   <varname>EXCLUDED</varname> columns has similar interactions.
> 
> How do you modify an EXCLUDED column?  Are you talking about a BEFORE
> INSERT trigger?  Reading the original text, I get the impression that
> it means "the behavior is obvious if you modify a column that is used
> with EXCLUDED in the DO UPDATE clause, but it can also happen if that
> column is not user with EXCLUDED".
> 
> Perhaps you should omit that sentence for clarity.

I think I found out what it trying to say by looking at the INSERT
manual page:

        Note that the effects of all per-row <literal>BEFORE
        INSERT</literal> triggers are reflected in
        <varname>excluded</varname> values, since those effects may
        have contributed to the row being excluded from insertion.

I modified the attached patch to explain this since it is not really the
same as modifying the actual row.  Does that add any value?  If not,
let's remove it.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: [DOCS] Confusing Trigger Docs.

From
"David G. Johnston"
Date:


On Wed, Nov 22, 2023 at 2:13 PM Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Nov 22, 2023 at 10:31:25AM +0100, Laurenz Albe wrote:
> I agree that the paragraph you are trying to improve needs it.
>
> I am not sure about that last sentence you added:
>
>   The modification of
>   <varname>EXCLUDED</varname> columns has similar interactions.
>
> How do you modify an EXCLUDED column?  Are you talking about a BEFORE
> INSERT trigger?  Reading the original text, I get the impression that
> it means "the behavior is obvious if you modify a column that is used
> with EXCLUDED in the DO UPDATE clause, but it can also happen if that
> column is not user with EXCLUDED".
>
> Perhaps you should omit that sentence for clarity.

I think I found out what it trying to say by looking at the INSERT
manual page:

        Note that the effects of all per-row <literal>BEFORE
        INSERT</literal> triggers are reflected in
        <varname>excluded</varname> values, since those effects may
        have contributed to the row being excluded from insertion.

I modified the attached patch to explain this since it is not really the
same as modifying the actual row.  Does that add any value?  If not,
let's remove it.


There is too much exposition drowning out the main purpose here which is to explain how the dual trigger situation introduced with on conflict gets handled.  The following is a more direct approach.

If an insert command contains an on conflict do update clause, before insert row triggers will be
applied to the proposed row before conflict detection.
If the update branch is taken, before update row triggers will also be applied.
Either an insert or an update after row trigger will fire for each row.
Before statement triggers fire for insertions first and then for updates, while
after statement triggers fire in the reverse order, updates and then inserts.
Statement triggers fire regardless if any rows were actually inserted or updated.


Tangentially, having the partition table content between this and the merge content seems odd.  There also seems to be room to integrate this and merge a bit better but that is beyond what I want to try right now.

David J.

Re: [DOCS] Confusing Trigger Docs.

From
Peter Geoghegan
Date:
On Wed, Nov 22, 2023 at 1:13 PM Bruce Momjian <bruce@momjian.us> wrote:
> I think I found out what it trying to say by looking at the INSERT
> manual page:
>
>         Note that the effects of all per-row <literal>BEFORE
>         INSERT</literal> triggers are reflected in
>         <varname>excluded</varname> values, since those effects may
>         have contributed to the row being excluded from insertion.
>
> I modified the attached patch to explain this since it is not really the
> same as modifying the actual row.  Does that add any value?  If not,
> let's remove it.

I don't understand where the confusion lies.

EXCLUDED.* is literally the row that we tried and failed to insert,
which quite naturally carries the effects of BEFORE ROW triggers. This
seems like the only behavior that makes any sense to me, even if it
can lead to confusing outcomes at times. It comes with an obvious
problem: EXCLUDED.* might not exactly match the row originally
proposed for insertion by the INSERT statement. This kind of confusion
can happen even when the table only has INSERT-type BEFORE ROW
triggers, though -- cases involving a mix of INSERT and UPDATE row
triggers are just a more elaborate version of the same confusion.

I don't think that your proposed wording for this is an improvement.
In particular, "it is possible for both row-level BEFORE INSERT and
BEFORE UPDATE triggers to be executed on the same row" leaves me
wondering what "the same row" refers to. Is it the row as it was prior
to the BEFORE triggers ran, after they ran, or something else? Or is
it more like the execution context of a single row?

--
Peter Geoghegan



Re: [DOCS] Confusing Trigger Docs.

From
Laurenz Albe
Date:
On Wed, 2023-11-22 at 14:49 -0800, Peter Geoghegan wrote:
> I don't think that your proposed wording for this is an improvement.

Well, the existing wording is impenetrable even for someone with some
PostgreSQL knowledge, like me.

Yours,
Laurenz Albe



Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
On Thu, Nov 23, 2023 at 08:36:34AM +0100, Laurenz Albe wrote:
> On Wed, 2023-11-22 at 14:49 -0800, Peter Geoghegan wrote:
> > I don't think that your proposed wording for this is an improvement.
> 
> Well, the existing wording is impenetrable even for someone with some
> PostgreSQL knowledge, like me.

I moved the parition pagagraph to a more logical location and tried to
clarify the new paragraph to be more targeted on the goal, patch
attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: [DOCS] Confusing Trigger Docs.

From
Laurenz Albe
Date:
On Fri, 2023-11-24 at 13:14 -0500, Bruce Momjian wrote:
> On Thu, Nov 23, 2023 at 08:36:34AM +0100, Laurenz Albe wrote:
> > On Wed, 2023-11-22 at 14:49 -0800, Peter Geoghegan wrote:
> > > I don't think that your proposed wording for this is an improvement.
> >
> > Well, the existing wording is impenetrable even for someone with some
> > PostgreSQL knowledge, like me.
>
> I moved the parition pagagraph to a more logical location and tried to
> clarify the new paragraph to be more targeted on the goal, patch
> attached.

I cannot tell if that covers everything that the original text (which
I failed to understand) did, but your wording makes sense to me.

Yours,
Laurenz Albe



Re: [DOCS] Confusing Trigger Docs.

From
Bruce Momjian
Date:
On Sun, Nov 26, 2023 at 08:59:02PM +0100, Laurenz Albe wrote:
> On Fri, 2023-11-24 at 13:14 -0500, Bruce Momjian wrote:
> > On Thu, Nov 23, 2023 at 08:36:34AM +0100, Laurenz Albe wrote:
> > > On Wed, 2023-11-22 at 14:49 -0800, Peter Geoghegan wrote:
> > > > I don't think that your proposed wording for this is an improvement.
> > > 
> > > Well, the existing wording is impenetrable even for someone with some
> > > PostgreSQL knowledge, like me.
> > 
> > I moved the partition paragraph to a more logical location and tried to
> > clarify the new paragraph to be more targeted on the goal, patch
> > attached.
> 
> I cannot tell if that covers everything that the original text (which
> I failed to understand) did, but your wording makes sense to me.

Patch applied to master.  It doesn't have everything that the previous
docs had, but I considered that an improvement.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.