Thread: [DOCS] Confusing Trigger Docs.
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?
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 +
[ 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 +
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
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
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
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
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.
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.
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
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
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
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
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.