public inbox for [email protected]  
help / color / mirror / Atom feed
From: jian he <[email protected]>
To: [email protected]
Cc: Zsolt Parragi <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Date: Thu, 4 Jun 2026 10:10:37 +0800
Message-ID: <CACJufxFARVrYtOOYqPGfq0S0RtAkuLtQt26=g99fpaNGXM8-Tg@mail.gmail.com> (raw)
In-Reply-To: <CAAAe_zAHAjTVGE9fAuTZ9df7Y7n7Zctn8WtEkUfytAu+VKQE7w@mail.gmail.com>
References: <CACJufxHJAr2FjbeB6ghg_-N5dxX5JVnjKSLOUxOyt4TeaAWQkg@mail.gmail.com>
	<CACJufxHQ8tDG09mXW=8xfEK2_h+By1zLgrFd0EcshHp-TDGiGw@mail.gmail.com>
	<CACJufxFu7Y4FhVkaKT2Kaj8ym2T5TcwN93cR_6h4x66iLrSZ-Q@mail.gmail.com>
	<CACJufxHMYq2ZqvOhoSKUmx+wZUo=HixjfgGKDJ6L4cdCvwh2VA@mail.gmail.com>
	<[email protected]>
	<CACJufxH5YDMAQ_KXaPJaC4Zt1i2HNSNaYoskzNDRjSrE_UWjKw@mail.gmail.com>
	<CAN4CZFM_8jL7Rf43zLa-9P2BwC+_H-oM9kYxVLGcwu83KC6faA@mail.gmail.com>
	<CACJufxGGXnRP+Em521KvPVOmnPtya2zpPqrTuYDmrSY=AFVmfw@mail.gmail.com>
	<CAN4CZFNPE_UeTEy5TH9wwc6mBAtX7vnBhLSL47jRiHXA=6yXjQ@mail.gmail.com>
	<CACJufxGTxbpbaQjUxc69-Dovx6+KJd=81T5111uizUrdsEkDNQ@mail.gmail.com>
	<CAN4CZFOdet_ZZEr+sZhTUiAd-fjOprhMXDhkjdhz48pEoJc9Pg@mail.gmail.com>
	<CACJufxEKK44FtxiShTZtPtCehwm47k5wB+RobuRCUJRa_b0owg@mail.gmail.com>
	<CACJufxFT-P-QZ-w5tVKOicWayUb1p407mWTDX5CGbf6XP2MGcA@mail.gmail.com>
	<CACJufxGhZy9s1ZZhOOu95AMpynk5acVxFVqHqoYcbbscezrbMA@mail.gmail.com>
	<CAAAe_zCcq2jGqNU7QMd0XjknG4r=+C4d5jK9nQJCL4Wi2W8L-Q@mail.gmail.com>
	<CAAAe_zAHAjTVGE9fAuTZ9df7Y7n7Zctn8WtEkUfytAu+VKQE7w@mail.gmail.com>

On Thu, May 7, 2026 at 9:38 AM Henson Choi <[email protected]> wrote:
>
> MINOR ISSUES
> ------------
>
> 1. Test: cross-file dependency on trigger_func (create_table_like.sql)
>
>    create_table_like.sql references trigger_func without creating it,
>    relying on triggers.sql having run first.  The dependency is noted
>    in a comment, and parallel_schedule guarantees the correct order for
>    a full "make check" run.  However, "make check TESTS=create_table_like"
>    will fail because trigger_func does not exist.
>
>    PostgreSQL convention is that each test file creates and drops the
>    objects it needs.  Please add a local definition of trigger_func in
>    create_table_like.sql (and drop it at the end of the new block).
>
OK.

> 2. Test: EXCLUDING TRIGGERS not exercised
>
>    The grammar now accepts EXCLUDING TRIGGERS, but no test uses it.
>    The default (no option) is equivalent, but a one-line smoke test
>    would confirm the keyword is accepted and has the expected effect:
>
>      CREATE TABLE t_excl (LIKE source_table EXCLUDING TRIGGERS);
>
OK.

> 3. Documentation: create_table.sgml wording inconsistent with
>    create_foreign_table.sgml
>
>    create_foreign_table.sgml already reads:
>
>      "All non-internal triggers are copied to the new table."
>
>    create_table.sgml reads:
>
>      "All non-internal triggers on the original table will be created
>       on the new table."
>
>    Recommended wording for create_table.sgml:
>
>      "All non-internal triggers are copied to the new table."
>
OK.

> 4. Code comment: capitalisation in generateClonedTriggerStmt()
>    (trigger.c)
>
>      /* Reconstruct trigger function String list */
>
>    "String" should be lowercase "string".
>
Other places also have "String list", it refers to the T_String node type.

> 5. Code comment: overly verbose in expandTableLikeClause()
>    (parse_utilcmd.c)
>
>      /* We make use of CreateTrigStmt's trigcomment option */
>
>    The code is self-explanatory.  I would recommend removing it or
>    replacing it with something like:
>
>      /* pass comment through to CreateTrigger */
>

Other places in parse_utilcmd.c have:
/*
 * We make use of IndexStmt's idxcomment option, so as not to
 * need to know now what name the index will have.
 */

/*
 * We make use of CreateStatsStmt's stxcomment option, so as
 * not to need to know now what name the statistics will have.
 */

We can change it to:
/*
 * We make use of CreateTrigStmt's trigcomment option,  so as
 * not to need to know now what name the triggers will have.
 */

>
> 7. Whole-row reference restriction: implementation gap or deliberate?
>
>    Triggers whose WHEN clause contains a whole-row reference (OLD.*,
>    NEW.*) are rejected.  Is this a deliberate decision, or a known gap
>    left for a follow-up?  If the latter, a XXX comment at the rejection
>    site would help future contributors:
>
>      /*
>       * XXX: whole-row Vars could in principle be handled by passing the
>       * target table's composite type OID as to_rowtype, but
>       * generateClonedTriggerStmt() currently has no access to it.
>       */
>

Other places already reject it (search found_whole_row in parse_utilcmd.c).
It cannot be supported because the source table's whole-row type
differs from the target table's whole-row type.

The v10 in https://www.postgresql.org/message-id/CACJufxEcKTa5DaDJS%3DZ25xezCEyuLbSzORDSmT4%3DjyZymsAK8A%40mail...
has addressed most of the issues mentioned above.

Only issue remaining is changing one of the comments to
/*
 * We make use of CreateTrigStmt's trigcomment option,  so as
 * not to need to know now what name the triggers will have.
 */






view thread (12+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
  In-Reply-To: <CACJufxFARVrYtOOYqPGfq0S0RtAkuLtQt26=g99fpaNGXM8-Tg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox