Re: COMMENTS are not being copied in CREATE TABLE LIKE

From: Tomas Vondra <tomas(at)vondra(dot)me>
To: Alex Liapychev <coder(dot)sam(at)gmail(dot)com>, jim(dot)jones(at)uni-muenster(dot)de
Cc: matheusssilv97 <matheusssilv97(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, "masao(dot)fujii" <masao(dot)fujii(at)gmail(dot)com>, "david(dot)g(dot)johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, tgl <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "huseyin(dot)d3r" <huseyin(dot)d3r(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, chochoforwork(at)gmail(dot)com, 44973863(at)qq(dot)com
Subject: Re: COMMENTS are not being copied in CREATE TABLE LIKE
Date: 2026-09-21 13:58:08
Message-ID: 223a6be3-3917-47a4-9b5d-51720c60ad58@vondra.me
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 9/20/26 22:28, Alex Liapychev wrote:
> Hello Jim,
> Hello everyone,
>
> Thank you Jim and all the reviewers for working on this patch.
>
> We reviewed the patch version v3 on September 20, 2026.
>
> The patch aims to fill the gap in CREATE TABLE LIKE functionality, where INCLUDING COMMENTS or INCLUDING ALL does not copy a comment of a Table itself.
> While there is still a discussion going on, if the table's own comment is worth copying at all, we concentrated on reviewing implementation itself.
>
> Patch set structure is good: description of what was done and why, documentation and tests included.
> We compiled the patch from branch cf/6482 (commit: 451748bf) against master (commit: 9e17d25e79d) on MacOS (version 26.6.2) on Intel hardware.
> Run standard tests(meson tests) and executed manual tests in psql. Tests in both master and cf/6482 completed successfully.
>
> Below are our findings:
>
> 1. Corner case: concatenation of big comments, whose total size exceeds MaxAllocSize, fails with an error.
> Here is the test case to prove that:
> ```
> CREATE TABLE comment_1gb_test (id BIGINT);
> -- set comment 1 GiB in size
> UPDATE pg_catalog.pg_description
> SET description = pg_catalog.repeat('x', 1000000000)
> WHERE classoid = 'pg_catalog.pg_class'::regclass
> AND objoid = 'public.comment_1gb_test'::regclass
> AND objsubid = 0;
> SELECT octet_length(description) as comment_size_bytes FROM pg_catalog.pg_description WHERE classoid = 'pg_catalog.pg_class'::regclass AND objoid = 'public.comment_1gb_test'::regclass AND objsubid = 0;
> comment_size_bytes
> --------------------
> 1000000000
> (1 row)
> CREATE TABLE xxl1 (LIKE comment_1gb_test INCLUDING ALL);
> CREATE TABLE xxl2 (LIKE comment_1gb_test INCLUDING ALL);
> ALTER TABLE xxl2 RENAME COLUMN id TO id2;
>
> CREATE TABLE merge_xxl (LIKE xxl1 INCLUDING ALL, LIKE xxl2 INCLUDING ALL);
> ERROR: string buffer exceeds maximum allowed length (1073741823 bytes)
> DETAIL: Cannot enlarge string buffer containing 1000000001 bytes by 1000000000 more bytes.
> ```
> Some form of truncation should be applied, cap the total size is easiest: first table's comment takes an advantage, others - as fit.
>

I'd just reject such cases, with an ERROR that says the comment would be
too long. It's cleaner than just silently start discarding user
information. The number of people hitting this is about 0 anyway. Who
would even have comments of this size?

That being said, I'm not convinced we actually want to concatenate
comments like this. It feels a bit weird, and it can probably lead to
weird stuff like "duplicate" comments, etc. Do we have any precedent for
this behavior? Are we concatenating comments (or other stuff) anywhere?
I couldn't find such place, but maybe I missed something.

FWIW if we really are worried about very long comments, then maybe
expandTableLikeClause should be more careful about freeing comments
during the concatenation? Right now it keeps all the comments and then
also the intermediate strings during concatenation. (I think.)

> 2. Code review notes:
> 2.1. nitpick: parse_utilcmd.c:46: order of includes would be better if added include ("lib/stringinfo.h") was placed either before "miscadmin.h" (alphabetical order) or before "utils/..." (functional order).

Before miscadmin.h, please. We keep includes in alphabetical order.

> 2.2. nitpick: parse_utilcmd.c:1652: it would match style of surrounding code better if local variable `CommentStmt *stmt` would be named `comment_stmt`; see code above in the same function: `stats_stmt` (line 1618), `index_stmt` (line 1577), etc.

Seems very cosmetic, and there's also a lot of places using 'stmt'.

> 2.3. nitpick: create_table_like.out:486 & create_table_like.sql:198: Since behaviour of INCLUDING ALL has also been changed by this patch. It would be better to update the tests to cover it.
>

Yes, that's a fair point. It'd be good to test INCLUDING ALL copies
comments too. The existing INCLUDING ALL test does not check that.

Aside from that, I don't understand why this patch needs to add
CREATE_TABLE_LIKE_COMMENTS to the last block in transformTableLikeClause
intended to deal with options that need column numbers. I mean, this
deals with a comment on the table itself, no? Or does it need to wait
for some other reason, and the comment is misleading?

regards

--
Tomas Vondra

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shihao zhong 2026-09-21 14:09:50 Re: [patch] Cache invalidation for I/O Workers
Previous Message Jingtang Zhang 2026-09-21 13:34:16 Re: [PATCH] Use bounded GIN pending-list cleanup in parallel autovacuum