Horton

Thread

Commits

  1. Doc: clarify the restrictions of AFTER triggers with transition tables.

  1. Document transition table triggers are not allowed on views/foreign tables

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-07-15T07:55:57Z

    Hi,

Another thing I noticed about transition tables is that while we
prohibit transition tables on views/foreign tables, there is no
description about that in the user-facing documentation.  So I would
like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
patch for that.

Best regards,
Etsuro Fujita

  2. Re: Document transition table triggers are not allowed on views/foreign tables

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-07-23T10:45:42Z

    On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> Another thing I noticed about transition tables is that while we
> prohibit transition tables on views/foreign tables, there is no
> description about that in the user-facing documentation.  So I would
> like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> patch for that.

If there are no objections, I will push this and back-patch it to all
supported versions.

Best regards,
Etsuro Fujita

  3. Re: Document transition table triggers are not allowed on views/foreign tables

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-07-23T15:06:32Z

    On Wed, Jul 23, 2025 at 4:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > Another thing I noticed about transition tables is that while we
> > prohibit transition tables on views/foreign tables, there is no
> > description about that in the user-facing documentation.  So I would
> > like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> > patch for that.

I think the restriction should be specified in a manner similar to how
restriction on CONSTRAINT option for foreign tables is specified i.e.
in " This option is only allowed for an AFTER trigger that is not a
constraint trigger; also, if the trigger is an UPDATE trigger, it must
not specify a column_name list.". But that sentence is already a bit
complex because of ; also, ... part. How about splitting the sentence
into two and mentioning restriction like below?

"This option is only allowed for an AFTER trigger on tables other than
views or foreign tables. The trigger should not be a constraint
trigger. If the trigger is an UPDATE trigger, it must not specify a
column_name list when using this option."

-- 
Best Wishes,
Ashutosh Bapat

  4. Re: Document transition table triggers are not allowed on views/foreign tables

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-07-25T09:52:22Z

    Hi Ashutosh,

On Thu, Jul 24, 2025 at 12:06 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> On Wed, Jul 23, 2025 at 4:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > Another thing I noticed about transition tables is that while we
> > > prohibit transition tables on views/foreign tables, there is no
> > > description about that in the user-facing documentation.  So I would
> > > like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> > > patch for that.
>
> I think the restriction should be specified in a manner similar to how
> restriction on CONSTRAINT option for foreign tables is specified i.e.
> in " This option is only allowed for an AFTER trigger that is not a
> constraint trigger; also, if the trigger is an UPDATE trigger, it must
> not specify a column_name list.". But that sentence is already a bit
> complex because of ; also, ... part. How about splitting the sentence
> into two and mentioning restriction like below?
>
> "This option is only allowed for an AFTER trigger on tables other than
> views or foreign tables. The trigger should not be a constraint
> trigger. If the trigger is an UPDATE trigger, it must not specify a
> column_name list when using this option."

Good idea!  This might be nitpicking, but one thing I noticed is this
part of the first sentence: "an AFTER trigger on tables other than
views or foreign tables".  Like the CONSTRAINT-restrictions
description above, how about just saying "an AFTER trigger on a plain
table (not a foreign table)"?  No need to mention views, so I removed
that.  I also changed to singular because that sounds natural.  My
first language is not English, though.  Other than that the change
looks good to me.

Thanks for the review!

Best regards,
Etsuro Fujita

  5. Re: Document transition table triggers are not allowed on views/foreign tables

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-07-25T10:59:00Z

    On Fri, Jul 25, 2025 at 3:22 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> Hi Ashutosh,
>
> On Thu, Jul 24, 2025 at 12:06 AM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > On Wed, Jul 23, 2025 at 4:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > > Another thing I noticed about transition tables is that while we
> > > > prohibit transition tables on views/foreign tables, there is no
> > > > description about that in the user-facing documentation.  So I would
> > > > like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> > > > patch for that.
> >
> > I think the restriction should be specified in a manner similar to how
> > restriction on CONSTRAINT option for foreign tables is specified i.e.
> > in " This option is only allowed for an AFTER trigger that is not a
> > constraint trigger; also, if the trigger is an UPDATE trigger, it must
> > not specify a column_name list.". But that sentence is already a bit
> > complex because of ; also, ... part. How about splitting the sentence
> > into two and mentioning restriction like below?
> >
> > "This option is only allowed for an AFTER trigger on tables other than
> > views or foreign tables. The trigger should not be a constraint
> > trigger. If the trigger is an UPDATE trigger, it must not specify a
> > column_name list when using this option."
>
> Good idea!  This might be nitpicking, but one thing I noticed is this
> part of the first sentence: "an AFTER trigger on tables other than
> views or foreign tables".  Like the CONSTRAINT-restrictions
> description above, how about just saying "an AFTER trigger on a plain
> table (not a foreign table)"?  No need to mention views, so I removed
> that.

I was actually going to suggest that, but I wasn't sure why you wanted
to mention "views" explicitly.

> I also changed to singular because that sounds natural.  My
> first language is not English, though.  Other than that the change
> looks good to me.

+1.

-- 
Best Wishes,
Ashutosh Bapat

  6. Re: Document transition table triggers are not allowed on views/foreign tables

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-07-26T11:29:19Z

    On Fri, Jul 25, 2025 at 7:59 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> On Fri, Jul 25, 2025 at 3:22 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Thu, Jul 24, 2025 at 12:06 AM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> > > On Wed, Jul 23, 2025 at 4:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > > On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > > > Another thing I noticed about transition tables is that while we
> > > > > prohibit transition tables on views/foreign tables, there is no
> > > > > description about that in the user-facing documentation.  So I would
> > > > > like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> > > > > patch for that.
> > >
> > > I think the restriction should be specified in a manner similar to how
> > > restriction on CONSTRAINT option for foreign tables is specified i.e.
> > > in " This option is only allowed for an AFTER trigger that is not a
> > > constraint trigger; also, if the trigger is an UPDATE trigger, it must
> > > not specify a column_name list.". But that sentence is already a bit
> > > complex because of ; also, ... part. How about splitting the sentence
> > > into two and mentioning restriction like below?
> > >
> > > "This option is only allowed for an AFTER trigger on tables other than
> > > views or foreign tables. The trigger should not be a constraint
> > > trigger. If the trigger is an UPDATE trigger, it must not specify a
> > > column_name list when using this option."
> >
> > Good idea!  This might be nitpicking, but one thing I noticed is this
> > part of the first sentence: "an AFTER trigger on tables other than
> > views or foreign tables".  Like the CONSTRAINT-restrictions
> > description above, how about just saying "an AFTER trigger on a plain
> > table (not a foreign table)"?  No need to mention views, so I removed
> > that.
>
> I was actually going to suggest that, but I wasn't sure why you wanted
> to mention "views" explicitly.
>
> > I also changed to singular because that sounds natural.  My
> > first language is not English, though.  Other than that the change
> > looks good to me.
>
> +1.

Cool!  I updated the patch as above.  I will push this version.

Best regards,
Etsuro Fujita

  7. Re: Document transition table triggers are not allowed on views/foreign tables

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-07-28T05:14:45Z

    On Sat, Jul 26, 2025 at 4:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Fri, Jul 25, 2025 at 7:59 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > On Fri, Jul 25, 2025 at 3:22 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > On Thu, Jul 24, 2025 at 12:06 AM Ashutosh Bapat
> > > <ashutosh.bapat.oss@gmail.com> wrote:
> > > > On Wed, Jul 23, 2025 at 4:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > > > On Tue, Jul 15, 2025 at 4:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > > > > Another thing I noticed about transition tables is that while we
> > > > > > prohibit transition tables on views/foreign tables, there is no
> > > > > > description about that in the user-facing documentation.  So I would
> > > > > > like to propose to do $SUBJECT in create_trigger.sgml.  Attached is a
> > > > > > patch for that.
> > > >
> > > > I think the restriction should be specified in a manner similar to how
> > > > restriction on CONSTRAINT option for foreign tables is specified i.e.
> > > > in " This option is only allowed for an AFTER trigger that is not a
> > > > constraint trigger; also, if the trigger is an UPDATE trigger, it must
> > > > not specify a column_name list.". But that sentence is already a bit
> > > > complex because of ; also, ... part. How about splitting the sentence
> > > > into two and mentioning restriction like below?
> > > >
> > > > "This option is only allowed for an AFTER trigger on tables other than
> > > > views or foreign tables. The trigger should not be a constraint
> > > > trigger. If the trigger is an UPDATE trigger, it must not specify a
> > > > column_name list when using this option."
> > >
> > > Good idea!  This might be nitpicking, but one thing I noticed is this
> > > part of the first sentence: "an AFTER trigger on tables other than
> > > views or foreign tables".  Like the CONSTRAINT-restrictions
> > > description above, how about just saying "an AFTER trigger on a plain
> > > table (not a foreign table)"?  No need to mention views, so I removed
> > > that.
> >
> > I was actually going to suggest that, but I wasn't sure why you wanted
> > to mention "views" explicitly.
> >
> > > I also changed to singular because that sounds natural.  My
> > > first language is not English, though.  Other than that the change
> > > looks good to me.
> >
> > +1.
>
> Cool!  I updated the patch as above.  I will push this version.

LGTM.

-- 
Best Wishes,
Ashutosh Bapat

  8. Re: Document transition table triggers are not allowed on views/foreign tables

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-08-02T09:50:48Z

    On Mon, Jul 28, 2025 at 2:14 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> On Sat, Jul 26, 2025 at 4:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > I updated the patch as above.  I will push this version.
>
> LGTM.

Pushed and back-patched.

Thanks!

Best regards,
Etsuro Fujita