Re: CREATE TABLE LIKE INCLUDING TRIGGERS

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Date: 2026-01-02 09:25:10
Message-ID: AF1AE30F-4379-462D-B321-FB1658230B5D@yandex-team.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Jian!

The feature makes sense from my POV.

> On 31 Dec 2025, at 10:08, jian he <jian(dot)universality(at)gmail(dot)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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Soumya S Murali 2026-01-02 10:09:40 Re: 001_password.pl fails with --without-readline
Previous Message Nicolas Adenis-Lamarre 2026-01-02 09:19:01 Re: Planner : anti-join on left joins