MIME-Version: 1.0
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> <AF1AE30F-4379-462D-B321-FB1658230B5D@yandex-team.ru>
In-Reply-To: <AF1AE30F-4379-462D-B321-FB1658230B5D@yandex-team.ru>
From: Robert Haas <robertmhaas@gmail.com>
Date: Tue, 13 Jan 2026 13:03:15 -0500
Message-ID: <CA+TgmoY8v44Y8j--b7e72kuvupZdtMEZf6FVjRfeqsG4gunpzw@mail.gmail.com>
Subject: Re: CREATE TABLE LIKE INCLUDING TRIGGERS
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: jian he <jian.universality@gmail.com>, 
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BTgmoY8v44Y8j--b7e72kuvupZdtMEZf6FVjRfeqsG4gunpzw%40mail.gmail.com>
Precedence: bulk

On Fri, Jan 2, 2026 at 4:25=E2=80=AFAM Andrey Borodin <x4mmm@yandex-team.ru=
> wrote:
> First two steps seems to add Oids along with RangeVars. It seems suspicio=
us to me.
> Parse Nodes seem to use "textual" identifiers without resolving them to O=
ids. Oids are specific to session catalog snapshot, but parse nodes
> By adding Oid fields we will have to check both RangeVars and Oids all ov=
er the code.
> Other INCLUDING options (indexes, constraints) don't modify their stateme=
nt nodes this way - they create fresh nodes with resolved references.

It seems like what generateClonedIndexStmt() does for the relation name is =
just:

        index->relation =3D 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.

--=20
Robert Haas
EDB: http://www.enterprisedb.com