public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey Borodin <[email protected]>
To: jian he <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
Date: Fri, 2 Jan 2026 14:25:10 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACJufxHMYq2ZqvOhoSKUmx+wZUo=HixjfgGKDJ6L4cdCvwh2VA@mail.gmail.com>
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>
Hi Jian!
The feature makes sense from my POV.
> On 31 Dec 2025, at 10:08, jian he <[email protected]> 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.
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox