Thread
-
CREATE TABLE LIKE INCLUDING TRIGGERS
jian he <jian.universality@gmail.com> — 2025-09-29T09:35:00Z
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. --------------- v1-0001: "refactor CreateTrigger and CreateTriggerFiringOn". Similar to CreateStatistics, some of the expressions stored in the catalog pg_trigger are already transformed, when we retrieve it as a base model for constructing a new CreateTrigStmt, we can not do parse analysis of it again. see transformStatsStmt for similar handling. The CreateTrigger function, (Node *whenClause) is always NULL, so I think it's safe to remove the argument whenClause. v1-0002: CREATE TABLE LIKE INCLUDING TRIGGERS. -
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
jian he <jian.universality@gmail.com> — 2025-10-01T09:28:24Z
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. -
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
jian he <jian.universality@gmail.com> — 2025-12-29T01:26:06Z
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] [1]: https://postgr.es/m/CACJufxGkqYrmwMdvUOUPet0443oUTgF_dKCpw3TfJiutfuywAQ@mail.gmail.com v3-0003 is for CREATE TABLE LIKE INCLUDING TRIGGERS. -- jian https://www.enterprisedb.com/ -
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
jian he <jian.universality@gmail.com> — 2025-12-31T05:08:24Z
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]: https://postgr.es/m/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/
-
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Henson Choi <assam258@gmail.com> — 2026-05-05T11:17:31Z
Hi Jian, Thank you for the patch. I applied v9 to a clean checkout, built it, and ran the regression suite — all tests pass. Nice work. I have attached a quality assessment report below. A line-by-line code review will follow in due course. Detailed gcov coverage in HTML format is included in coverage.tgz. CREATE TABLE LIKE INCLUDING TRIGGERS — Coverage Review ====================================================== Patch: CREATE TABLE LIKE INCLUDING TRIGGERS Commitfest: https://commitfest.postgresql.org/patch/6087 Review Methodology: Quality assessment: build verification, regression testing, and code coverage analysis (gcov, modified lines only). This is not a full code review. Limitations: - Not a security audit or formal verification TABLE OF CONTENTS ----------------- 1. Executive Summary 2. Issues Found 2.1 Critical / Major 2.2 Minor (Review Needed) 3. Test Coverage 3.1 Covered Areas 3.2 Untested Items 4. Code Coverage Analysis 4.1 Overall Coverage 4.2 Uncovered Code Risk Assessment 5. Commit Analysis 6. Recommendations 7. Conclusion 1. EXECUTIVE SUMMARY -------------------- Overall assessment: GOOD The patch adds INCLUDING TRIGGERS to CREATE TABLE LIKE (and CREATE FOREIGN TABLE LIKE). Build succeeds and all regression tests pass. The following behaviors are verified by the test suite: - Internal triggers (FK-associated) are not copied - INSTEAD OF triggers on views produce an error - Whole-row references in WHEN clauses produce an error - Trigger comments are copied only when INCLUDING COMMENTS is given - The trigger's enabled state is preserved on the new table Rating by Area: Test Coverage: Good (broad scenario coverage) Documentation: Thin (functional but sparse) Build/Regress: Pass 2. ISSUES FOUND --------------- 2.1 Critical / Major None. 2.2 Minor (Review Needed) #1 [Documentation] INCLUDING TRIGGERS entry is too brief create_table.sgml currently says only: "All non-internal triggers on the original table will be created on the new table." The following behaviors are not mentioned: (a) The trigger's enabled state is preserved on the new table. (b) A WHEN clause with a whole-row reference causes an error. (c) INSTEAD OF triggers cannot be copied to a plain table. Suggested addition after the existing sentence: <para> The enabled state of each trigger (as set by ALTER TABLE ... ENABLE/DISABLE TRIGGER) is preserved on the new table. An error is raised if any trigger's WHEN clause contains a whole-row table reference. INSTEAD OF triggers on views cannot be copied to a plain table. </para> The same applies to the entry in create_foreign_table.sgml. 3. TEST COVERAGE ---------------- 3.1 Covered Areas triggers.sql additions exercise: - Whole-row WHEN clause (OLD IS NOT NULL) → error - Whole-row WHEN clause (NEW IS NOT NULL) → error - Basic LIKE INCLUDING TRIGGERS + INCLUDING COMMENTS - Trigger enabled state: ENABLE REPLICA, DISABLE, ENABLE ALWAYS - INSTEAD OF triggers on a view → error - Statement-level view triggers with transition tables - Partitioned table: table-level trigger only, not partition triggers - Transition table triggers on partitioned table copy - Cross-schema trigger function - Constraint trigger with FROM clause create_table_like.sql additions exercise: - Triggers with WHEN clause and column-specific UPDATE (via LIKE ctl_table INCLUDING ALL on a foreign table) 3.2 Untested Items The following lines were uncovered per gcov analysis (see section 4). Line Code path ------------------------------------------------------------------------------- trigger.c:6959 elog: trigger OID not found in pg_trigger trigger.c:6966 elog: trigger function OID not in syscache trigger.c:7002 elog: tgargs is null when tgnargs > 0 parse_utilcmd.c:1640 continue: skip internal trigger Lines 6959, 6966, 7002 are catalog-inconsistency guards. All three require catalog corruption to reach; untestable under normal operation. No functional risk. Line 1640 (the internal-trigger skip) is reachable but requires a source table that has FK constraints (which create internal triggers). The patch description explicitly states that FK-associated internal triggers are excluded, but no test verifies this behavior. Suggested test (add near the tgenabled block in triggers.sql): -- Internal triggers (e.g. FK) must not be copied CREATE TABLE fk_parent (id int PRIMARY KEY); CREATE TABLE fk_child (id int REFERENCES fk_parent(id)); CREATE TABLE fk_child_copy (LIKE fk_child INCLUDING TRIGGERS); SELECT count(*) FROM pg_trigger WHERE tgrelid = 'fk_child_copy'::regclass; -- expect 0 DROP TABLE fk_child_copy, fk_child, fk_parent; 4. CODE COVERAGE ANALYSIS ------------------------- 4.1 Overall Coverage (modified lines only) trigger.c 96.8% (91 / 94 lines) parse_utilcmd.c 92.3% (12 / 13 lines) ----------------------------------------------- Combined 96.8% (120 / 124 lines) 4.2 Uncovered Code Risk Assessment trigger.c:6959, 6966, 7002 — catalog-inconsistency guards. All three require catalog corruption to reach; untestable under normal operation. No functional risk. parse_utilcmd.c:1640 — internal trigger skip. Reachable with a table that has FK constraints. A test is straightforward; see section 3.2 for sample SQL. 5. COMMIT ANALYSIS ------------------ 2 commits: Commit Area Files +/- Key Content ----------------------------------------------------------------------- e73f551 Core feature 14 +531/-19 generateClonedTriggerStmt, expandTableLikeClause, grammar, docs, tests 39ae762 tgenabled copy 8 +27/-6 trigger enabled state preserved on new table The two-commit structure is sensible given that the enabled state copy was a post-review addition. Squashing is straightforward if preferred. 6. RECOMMENDATIONS ------------------ 1. Add a test verifying that FK internal triggers are not copied (sample SQL in section 3.2). 2. Expand the INCLUDING TRIGGERS documentation entry (section 2.2 #1). 7. CONCLUSION ------------- From a quality standpoint, the patch is in good shape: regression tests pass, coverage is at 96.8% on modified lines, and the key scenarios (enabled state, whole-row errors, partitioned tables, cross-schema functions) are all exercised by the test suite. The missing test for internal trigger exclusion (#1) and the documentation gaps (#2) are worth addressing before commit. The remaining items are minor. -
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Henson Choi <assam258@gmail.com> — 2026-05-07T01:38:35Z
Hi Jian, This is the line-by-line code review I promised earlier. The overall conclusion is that the patch is in good shape. I have some minor items below, none of which are blockers. WHAT LOOKS GOOD --------------- The keyword placement is correct. TRIGGERS is classified as UNRESERVED_KEYWORD and also listed in bare_label_keyword, so existing SQL that uses "triggers" as an identifier continues to work without quoting. No compatibility concern here. generateClonedTriggerStmt() is well-structured. The WHEN-clause remapping via map_variable_attnos() and the transformed flag to prevent re-transformation in CreateTriggerFiringOn() are correct. pg_dump needs no changes. LIKE-copied triggers are stored as independent pg_trigger rows, so the existing dump logic picks them up automatically. 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). 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); 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." Two problems with the latter: a) "on the original table" is redundant; no other INCLUDING option includes this phrase (compare INCLUDING STATISTICS: "Extended statistics are copied to the new table."). b) "will be created" misrepresents the action; the correct verb is "are copied", matching the pattern used elsewhere. Recommended wording for create_table.sgml: "All non-internal triggers are copied to the new table." 4. Code comment: capitalisation in generateClonedTriggerStmt() (trigger.c) /* Reconstruct trigger function String list */ "String" should be lowercase "string". 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 */ 6. Commit message grammar (commit e73f551) Several sentences contain grammatical errors: a) "This will copy all source table's trigger to the new table." -> "...triggers..." b) "Internal trigger (such as foreign key associated trigger) won't being copied to new table." -> "Internal triggers (such as foreign-key-associated triggers) won't be copied to the new table." c) "However this command will fail if the source table's trigger contain whole-row reference." -> "However, this command will fail if the source table's triggers contain a whole-row reference." 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. */ SUMMARY OF ITEMS TO ADDRESS ---------------------------- [1] create_table_like.sql: define and drop trigger_func locally so the file can be run in isolation. [2] Add a test for EXCLUDING TRIGGERS. [3] create_table.sgml: align the INCLUDING TRIGGERS description with create_foreign_table.sgml, which already reads correctly: "All non-internal triggers are copied to the new table." [4] trigger.c generateClonedTriggerStmt(): lowercase "string" in comment "/* Reconstruct trigger function String list */". [5] parse_utilcmd.c expandTableLikeClause(): simplify or remove the comment "/* We make use of CreateTrigStmt's trigcomment option */". [6] Commit message: fix grammar in items (a), (b), (c) listed above. Items [3]-[6] are purely cosmetic. [1] and [2] are worth considering before commit, but none of these are blockers. Thanks for the patch. Best regards, Henson Choi -
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
lakshmi <lakshmigcdac@gmail.com> — 2026-05-27T06:03:29Z
> > Hi Jian, > > I applied the latest v9 patches on current master and tested them locally. > > The patches applied cleanly, PostgreSQL built successfully. > > I also did some manual testing for INCLUDING TRIGGERS. Trigger definitions > were copied correctly to the new table, and the trigger enabled/disabled > state (tgenabled) was also preserved properly after applying the second > patch. > > Everything worked as expected in my testing. > > Thanks for working on this feature. > > Regards, > Lakshmi G >
-
Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-05-27T22:53:13Z
Hello! Sorry for the late reply, somehow I missed the previous updates. I found one more problematic scenario, I think the patch should ignore INSTEAD OF triggers: CREATE TABLE base (id int, val text); CREATE VIEW v_instead AS SELECT id, val FROM base; CREATE FUNCTION v_instead_ins() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN INSERT INTO base VALUES (NEW.id, NEW.val); RETURN NEW; END; $$; CREATE TRIGGER v_instead_trg INSTEAD OF INSERT ON v_instead FOR EACH ROW EXECUTE FUNCTION v_instead_ins(); -- currently fails -- ERROR: "t_all" is a table -- DETAIL: Tables cannot have INSTEAD OF triggers. CREATE TABLE t_all (LIKE v_instead INCLUDING ALL); And also, either I don't understand something here, or the diff can be simplified a bit: - /* Transform expression. Copy to be sure we don't modify original */ - whenClause = transformWhereClause(pstate, - copyObject(stmt->whenClause), - EXPR_KIND_TRIGGER_WHEN, - "WHEN"); - /* we have to fix its collations too */ - assign_expr_collations(pstate, whenClause); + if (stmt->transformed) + whenClause = stmt->whenClause; + else + { + /* Transform expression. Copy to be sure we don't modify original */ + whenClause = transformWhereClause(pstate, + copyObject(stmt->whenClause), + EXPR_KIND_TRIGGER_WHEN, + "WHEN"); + + /* we have to fix its collations too */ + assign_expr_collations(pstate, whenClause); + + stmt->transformed = true; + } Do we need the last assignment in this diff? It sets stmt->transformed, but we don't actually transform the statement, we create a copy. The flag also doesn't seem to be used after that. Everything seems to work fine if I remove this assignment, and we don't need the const related function signature changes without it.