CREATE TABLE LIKE INCLUDING TRIGGERS

On Mon, Sep 29, 2025 at 5:35 PM jian he <jian.universality@gmail.com> wrote:

hi.

poc demo:
CREATE TABLE main_table (a int, b int);
CREATE FUNCTION trigger_func() RETURNS trigger LANGUAGE plpgsql AS '
BEGIN
RAISE NOTICE ''trigger_func(%) called: action = %, when = %, level = %'',
TG_ARGV[0], TG_OP, TG_WHEN, TG_LEVEL;
RETURN NULL;
END;';
CREATE TRIGGER modified_a BEFORE UPDATE OF a ON main_table
FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE trigger_func('modified_a');

CREATE TABLE main_table1(LIKE main_table INCLUDING TRIGGERS INCLUDING COMMENTS);
\d main_table1
Table "public.main_table1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Triggers:
modified_a BEFORE UPDATE OF a ON main_table1 FOR EACH ROW WHEN
(old.a <> new.a) EXECUTE FUNCTION trigger_func('modified_a')

foreign key associated internal triggers won't be copied to the new table.
source table trigger associated comment will be copied to the new table,
if INCLUDING COMMENTS is specified.

per
https://api.cirrus-ci.com/v1/artifact/task/5194724417470464/testrun/build/testrun/regress/regress/regression.diffs

there are many table name as t1 in regress test, add tests like
""CREATE TABLE t1 (a int, b text, c int);"
may result in error
+ERROR: relation "t1" already exists
in some OS.
So I changed the table name to avoid parallel regess test failure.

hi.

in CreateTrigger, we have comments:

* relOid, if nonzero, is the relation on which the trigger should be
* created. If zero, the name provided in the statement will be looked up.
*
* refRelOid, if nonzero, is the relation to which the constraint trigger
* refers. If zero, the constraint relation name provided in the statement
* will be looked up as needed.

We can put these two parameters into the CreateTrigStmt.
change it from
CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid,
Oid funcoid, Oid parentTriggerOid, Node *whenClause,
bool isInternal, bool in_partition)
to:

CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid constraintOid, Oid indexOid,
Oid funcoid, Oid parentTriggerOid, Node *whenClause,
bool isInternal, bool in_partition)

This is needed, ProcessUtilitySlow->CreateTrigger don't have the new
target table relation OID information, using CreateTrigStmt.relation
would cause repeated name lookup issue.

v3-0001 and v3-0002 refactor the CreateTrigger function.
The parameters relOid and refRelOid are removed and instead added to the
CreateTrigStmt structure.

These two patch (v3-0001, v3-0002) will also be used in [1]/messages/by-id/CACJufxGkqYrmwMdvUOUPet0443oUTgF_dKCpw3TfJiutfuywAQ@mail.gmail.com
[1]: /messages/by-id/CACJufxGkqYrmwMdvUOUPet0443oUTgF_dKCpw3TfJiutfuywAQ@mail.gmail.com

v3-0003 is for CREATE TABLE LIKE INCLUDING TRIGGERS.

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

On Mon, Dec 29, 2025 at 9:26 AM jian he <jian.universality@gmail.com> wrote:

v3-0001 and v3-0002 refactor the CreateTrigger function.
The parameters relOid and refRelOid are removed and instead added to the
CreateTrigStmt structure.

These two patch (v3-0001, v3-0002) will also be used in [1]
[1]: /messages/by-id/CACJufxGkqYrmwMdvUOUPet0443oUTgF_dKCpw3TfJiutfuywAQ@mail.gmail.com

v3-0003 is for CREATE TABLE LIKE INCLUDING TRIGGERS.

hi.

https://cirrus-ci.com/build/6583555523870720
https://api.cirrus-ci.com/v1/artifact/task/5664491007901696/testrun/build/testrun/pg_upgrade/002_pg_upgrade/data/regression.diffs

only FreeBSD fails. I suspect this is because pstrdup was not used in
generateClonedTriggerStmt.

now using pstrdup in generateClonedTriggerStmt; otherwise v4-0003 is identical
to v3-0003.

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

Hi Jian!

The feature makes sense from my POV.

On 31 Dec 2025, at 10:08, jian he <jian.universality@gmail.com> wrote:

<v4-0001-add-relOid-field-to-CreateTrigStmt.patch><v4-0002-add-constrrelOid-field-to-CreateTrigStmt.patch>

I'm not an expert in the area, but still decided to review the patch a bit.

First two steps seems to add Oids along with RangeVars. It seems suspicious to me.
Parse Nodes seem to use "textual" identifiers without resolving them to Oids. Oids are specific to session catalog snapshot, but parse nodes
By adding Oid fields we will have to check both RangeVars and Oids all over the code.
Other INCLUDING options (indexes, constraints) don't modify their statement nodes this way - they create fresh nodes with resolved references.

I'm not opposed, but I suggest you to get an opinion of an expert in parse nodes about it, maybe in Discord if this thread does not attract attention. It seems a fundamental stuff for two of your patchsets.

+ char *trigcomment; /* comment to apply to trigger, or NULL */
No other Create*Stmt has a comment field. Comments seem to be handled through separate CommentStmt creation.

Some nitpicking about tests:
1. INSTEAD OF triggers on views - The error is tested, but should also test that statement-level VIEW triggers work
2. Triggers on partitioned tables - What happens when you LIKE a partitioned table? Are partition triggers cloned?
3. Cross-schema trigger functions - The function name reconstruction handles schemas, but is it tested?

+ funcname = list_make2(makeString(schemaname),makeString(NameStr(procform->proname)));
Other NameStr() are pstrdup()'d, maybe let's pstrdup() this too?

+ /* Reconstruct trigger old transition table */
Second instance of this comment is wrong.

+ PG_KEYWORD("triggers", TRIGGERS, UNRESERVED_KEYWORD, BARE_LABEL)
Won't this break some user SQLs?

Thanks!

Best regards, Andrey Borodin.

On Fri, Jan 2, 2026 at 4:25 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

First two steps seems to add Oids along with RangeVars. It seems suspicious to me.
Parse Nodes seem to use "textual" identifiers without resolving them to Oids. Oids are specific to session catalog snapshot, but parse nodes
By adding Oid fields we will have to check both RangeVars and Oids all over the code.
Other INCLUDING options (indexes, constraints) don't modify their statement nodes this way - they create fresh nodes with resolved references.

It seems like what generateClonedIndexStmt() does for the relation name is just:

index->relation = heapRel;

This is not a "resolved reference", which I would understand to mean
an OID in this context. In fact, if heapRel->schemaname were ever NULL
here, this would be at least a bug and possibly a security issue.
However, I don't think that ever happens. ProcessUtilitySlow() calls
transformCreateStmt(), which forces the RangeVar to be
fully-qualified, and then ProcessUtilitySlow() fishes out the
transformed RangeVar so that it gets passed through to
expandTableLikeClause(), which in turn passes it through to
generateClonedIndexStmt() and generateClonedExtStatsStmt(). Similarly
constraints are handled by using that same transformed (i.e. forcibly
schema-qualified) RangeVar in the AlterTableStmt we construct.

To be honest, I find this all incredibly ugly. I think that
translating from names to OIDs and back to names so that we can later
translate back to OIDs is a horrendous, bug-prone mess. Nor is the
problem confined to table names. For example,
generateClonedIndexStmt() needs to translate the AM name and
tablespace name and any column names back to textual format so that
when we actually create the index we can redo the translation in the
forward direction and hopefully get the right answer.

But having said that it's incredibly ugly and I don't like anything
about it, it also does seem to be the established pattern. As you say,
it seems as though the patch makes CREATE LIKE ... (INCLUDING
TRIGGERS) work differently from other cases, and I'm guessing that's
not what we want to do. It's better to use the same ugly hack
consistently than to use different ugly hacks in different places.
Then if somebody ever decides to try to clean this up, they can clean
up all the cases in the same way.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Fri, Jan 2, 2026 at 5:25 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

+ char *trigcomment; /* comment to apply to trigger, or NULL */
No other Create*Stmt has a comment field. Comments seem to be handled through separate CommentStmt creation.

See CreateStatsStmt.stxcomment, IndexStmt.idxcomment.
We need CreateTrigStmt.trigcomment, because if INCLUDING COMMENTS is specified,
CreateTrigStmt.trigcomment can be used to hold the source object's comments.

Some nitpicking about tests:
1. INSTEAD OF triggers on views - The error is tested, but should also test that statement-level VIEW triggers work

ok. test added.

2. Triggers on partitioned tables - What happens when you LIKE a partitioned table? Are partition triggers cloned?

no. only the trigger on the partitioned table itself will be cloned.
see tests ``create table parted_constr_copy (like parted_constr
including all);``

3. Cross-schema trigger functions - The function name reconstruction handles schemas, but is it tested?

ok. test added.

+ funcname = list_make2(makeString(schemaname),makeString(NameStr(procform->proname)));
Other NameStr() are pstrdup()'d, maybe let's pstrdup() this too?

ok.

+ /* Reconstruct trigger old transition table */
Second instance of this comment is wrong.

ok.

+ PG_KEYWORD("triggers", TRIGGERS, UNRESERVED_KEYWORD, BARE_LABEL)
Won't this break some user SQLs?

it's marked as an un-reserved word, so it won't break any SQL, i think.

v4-0001, v4-0002 was removed, as Robert said in [1], now I am using
the same ugly
hack consistently, now code is more aligned with INCLUDING INDEXES, INCLUDING
STATISTICS.

pstrdup have been used in more places in generateClonedTriggerStmt.

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

On 16 Jan 2026, at 12:34, jian he <jian.universality@gmail.com> wrote:

IMO the patch is ready for committer.

Best regards, Andrey Borodin.

Hello!

I tested the patch, it works as described, but I did notice one possible issue:

Shouldn't this preserve the enabled state of the triggers, or if it
doesn't, should the documentation include this limitations?

Currently the new table will always use the default
TRIGGER_FIRES_ON_ORIGIN, regardless how it is configured for the
original table.

diff --git a/src/test/regress/sql/triggers.sql
b/src/test/regress/sql/triggers.sql
index a1e9dbba8bd..24f7bfb5837 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -260,6 +260,18 @@ ON      pc.oid = pd.tgrelid AND pd.tgname =
'before_ins_stmt_trig'
 ORDER BY 1;
 COMMENT ON TRIGGER before_ins_stmt_trig ON main_table IS NULL;

+-- Test that trigger firing state is not preserved by LIKE INCLUDING TRIGGERS
+ALTER TABLE main_table DISABLE TRIGGER before_ins_stmt_trig;
+CREATE TABLE main_table2 (LIKE main_table INCLUDING TRIGGERS);
+SELECT c.relname, t.tgname, t.tgenabled
+FROM pg_trigger t
+JOIN pg_class c ON t.tgrelid = c.oid
+WHERE t.tgname = 'before_ins_stmt_trig'
+  AND c.relname IN ('main_table', 'main_table2')
+ORDER BY c.relname;
+DROP TABLE main_table2;
+ALTER TABLE main_table ENABLE TRIGGER before_ins_stmt_trig;
+
 --
 -- Test case for bug with BEFORE trigger followed by AFTER trigger with WHEN
 --

On Wed, Jan 21, 2026 at 8:13 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:

Hello!

I tested the patch, it works as described, but I did notice one possible issue:

Shouldn't this preserve the enabled state of the triggers, or if it
doesn't, should the documentation include this limitations?

I intended to document it as
<para>
All non-internal triggers on the original table will be
created on the new table.
By default, these triggers fire in "origin" and "local" modes. Refer
to <link linkend="sql-altertable"><command>ALTER
TABLE</command></link>
to modify the firing condition.
</para>

what do you think?

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

Shouldn't this preserve the enabled state of the triggers, or if it
doesn't, should the documentation include this limitations?

I intended to document it as ...

After looking into this a bit more, I am more on the side of copying
this setting properly.

The already existing INCLUDING CONSTRAINTS copies the constraints,
including their enabled/disabled status, correctly marking them
disabled if a CHECK constraint is defined but not enforced. Wouldn't
it be strange for INCLUDING TRIGGERS to work differently?

From the test suite:

CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING
COMMENTS) INHERITS (ctlt1);
\d+ ctlt1_inh
Table "public.ctlt1_inh"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+------+-----------+----------+---------+----------+--------------+-------------
a | text | | not null | | main | | A
b | text | | | | extended | | B
Check constraints:
"cc" CHECK (length(b) > 100)
"ctlt1_a_check" CHECK (length(a) > 2)
"ctlt1_b_check" CHECK (length(b) > 100) NOT ENFORCED
Not-null constraints:
"ctlt1_a_not_null" NOT NULL "a" (local, inherited)
Inherits: ctlt1

On Thu, Jan 22, 2026 at 10:20 PM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:

After looking into this a bit more, I am more on the side of copying
this setting properly.

The already existing INCLUDING CONSTRAINTS copies the constraints,
including their enabled/disabled status, correctly marking them
disabled if a CHECK constraint is defined but not enforced. Wouldn't
it be strange for INCLUDING TRIGGERS to work differently?

hi.
please check the attached.

v6-0001 is the same as v5-0001.
v6-0002 ensures that CREATE TABLE ... LIKE ... INCLUDING TRIGGERS
correctly copies the tgenabled status from the source table's triggers
to the new table

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

This looks good to me, I only have two minor comments:

1. The test should drop main_table1 and some_t1

2. there's a missing word in a comment (do -> to do)

+ char    *trigcomment; /* comment to apply to trigger, or NULL */
+ bool transformed; /* true means no need do parse analysis for *
+ * whenClause */

On Fri, Feb 6, 2026 at 10:49 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:

This looks good to me, I only have two minor comments:

1. The test should drop main_table1 and some_t1

2. there's a missing word in a comment (do -> to do)

+ char    *trigcomment; /* comment to apply to trigger, or NULL */
+ bool transformed; /* true means no need do parse analysis for *
+ * whenClause */

hi.
The attached have addressed these two points.

I still keep two separate patches, in case there are differing opinions about
whether to copy the old table’s trigger tgenabled to the new table.
Squashing them into a single patch is quite straightforward.

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

Hi.

Minor rebase due to changes in
https://git.postgresql.org/cgit/postgresql.git/commit/?id=b2ff2a0b52962dcbcbc9c5a8d74fd4e8c28d1f2a

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

Hi.
rebase due to changes in
https://git.postgresql.org/cgit/postgresql.git/commit/?id=7724cb9935a96eabba80f5e62ee4b32068967dd2
https://git.postgresql.org/cgit/postgresql.git/commit/?id=fba4233c832870c8363438419743c48fdcb2151c

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