Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
From: Andrey Borodin <x4mmm@yandex-team.ru>
In-Reply-To: <CACJufxHMYq2ZqvOhoSKUmx+wZUo=HixjfgGKDJ6L4cdCvwh2VA@mail.gmail.com>
Date: Fri, 2 Jan 2026 14:25:10 +0500
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <AF1AE30F-4379-462D-B321-FB1658230B5D@yandex-team.ru>
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>
To: jian he <jian.universality@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/AF1AE30F-4379-462D-B321-FB1658230B5D%40yandex-team.ru>
Precedence: bulk

Hi Jian!

The feature makes sense from my POV.

> On 31 Dec 2025, at 10:08, jian he <jian.universality@gmail.com> wrote:
>=20
> =
<v4-0001-add-relOid-field-to-CreateTrigStmt.patch><v4-0002-add-constrrelOi=
d-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 =3D =
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.=