Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS

From: Andrey Borodin <[email-masked]>
Date: Fri, 2 Jan 2026 14:25:10 +0500

Hi Jian!

The feature makes sense from my POV.

> On 31 Dec 2025, at 10:08, jian he <[email-masked]> 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.

---

From: Robert Haas <[email-masked]>
Date: Tue, 13 Jan 2026 13:03:15 -0500

> 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