Re: CREATE TABLE LIKE INCLUDING TRIGGERS

On Fri, Jul 3, 2026 at 2:28 PM Henson Choi <assam258(at)gmail(dot)com> wrote:
>
> I've grouped them by how you might want to act on each.
>
> 1. Docs / typos (all cosmetic)
>

Typo fixed.

> - 0002: create_table.sgml's INCLUDING TRIGGERS entry doesn't mention that a
> trigger's enabled/disabled (tgenabled) state is preserved. Suggested,
> after the source-view sentence ("... are not copied."):
> "The enabled state of each trigger (as set by ALTER TABLE ...
> ENABLE/DISABLE TRIGGER) is preserved on the new table."
>

I changed it to:
+ The enabled state of each trigger (references <link
linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link>.<structfield>tgenabled</structfield>)
+ is preserved on the new table.

> 2. Code -- two fixes to consider
>
> (a) Concurrent trigger add can fail CREATE TABLE via a duplicate (reproduced)
>
> expandTableLikeClause() walks the source trigger array (trigdesc) by index,
> calling generateClonedTriggerStmt(), whose catalog access can process an
> invalidation and rebuild the source trigdesc. The array is sorted by tgname,
> so if another session adds a lower-sorting trigger mid-walk, the indexes shift
> and an already-processed trigger gets cloned again.
>
> I reproduced it by putting a sleep in the loop: with t100/t101/t102 on the
> source, adding t000 from another session right after t100 was processed
> (CREATE TRIGGER is compatible with AccessShareLock, so it isn't blocked) made
> the next iteration see the rebuilt array [t000, t100, t101, t102], with nt=1
> pointing at t100 again -- so it tried to create t100 twice on the new table
> and the whole CREATE TABLE failed with "trigger ... already exists".
>
> The removal side (DROP/ALTER/RENAME TRIGGER) is blocked by AccessExclusiveLock
> against the source's AccessShareLock, so the array can't shrink and nothing is
> missed; the only thing that can run concurrently is CREATE TRIGGER (a
> compatible lock), leaving just this duplicate. It's probably a rare path, but
> it seems worth addressing.
>

In v11, I changed it to first collect the trigger OIDs to be copied to the
target relation, and then call generateClonedTriggerStmt for each trigger OID.

> (b) tgenabled channel unification (CreateTriggerFiringOn)
>
> 0002 adds a tgenabled field to CreateTrigStmt and sets it in several places,
> but the deferred-unique trigger in index.c and the FK triggers in
> tablecmds.c still call CreateTrigger(). CreateTrigger() hardcodes tgenabled
> to TRIGGER_FIRES_ON_ORIGIN internally, so the trigger->tgenabled just
> assigned is a dead store. It's harmless today since the value is always
> ORIGIN, but it's a field set and then ignored. Unifying those sites on
> CreateTriggerFiringOn(..., trigger->tgenabled) (or dropping the assignment)
> would consolidate the two channels.
>

CreateTrigger change to return
```
CreateTriggerFiringOn(stmt, queryString, relOid, refRelOid,
constraintOid, indexOid, funcoid,
parentTriggerOid, whenClause, isInternal,
in_partition, stmt->tgenabled);
```
should be fine.

I also combined two patches (v10-0001, v10-0002) into one (v11-0001).

--
jian
https://www.enterprisedb.com/