From: Nathan Bossart <nathandbossart@gmail.com>
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Euler Taveira <euler@eulerto.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jonathan S. Katz <jkatz@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
Date: Tue, 22 Apr 2025 12:21:01 -0500
Message-ID: <aAfP_bitjVC1bcNV@nathan> (raw)
In-Reply-To: <CABdArM6j03AAX5A2B9TPCZrY1xr+eFpGpzfdOBAww7mZnniUFg@mail.gmail.com>
References: <Z-_sixINC08J_SQB@nathan>
<CAA4eK1LeMM8royHj5UyMBDbspD7+_70AJkV4Uji122hmkn7jOw@mail.gmail.com>
<Z_WGI4cKoTjhUXrs@nathan>
<4c2a6100-738e-48cc-8a11-2d85f95ae0ed@app.fastmail.com>
<Z_X1jfsMrBBNUx4n@paquier.xyz>
<Z_bFDNzcqWxatpL-@nathan>
<18a111db-9f31-449c-a14b-61779a69f344@app.fastmail.com>
<Z_mHvqVyGjEZhzl7@nathan>
<aAZqiKRhUv7-fSpQ@nathan>
<CABdArM6j03AAX5A2B9TPCZrY1xr+eFpGpzfdOBAww7mZnniUFg@mail.gmail.com>
On Tue, Apr 22, 2025 at 05:17:17PM +0530, Nisha Moond wrote:
> Thanks for the patch! I tested it for functionality and here are a few comments:
Thank you for reviewing.
> 1) While testing, I noticed an unexpected behavior with the new 512
> byte length restriction in place. Since there´s no explicit
> restriction on the column roname´s type, it´s possible to insert an
> origin name exceeding 512 bytes. For instance, can use the COPY
> command -- similar to how pg_dump might dump and restore catalog
> tables.
>
> For example:
> postgres=# COPY pg_catalog.pg_replication_origin (roident, roname) FROM stdin;
> Enter data to be copied followed by a newline.
> End with a backslash and a period on a line by itself, or an EOF signal.
> >> 44 regress_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa... [>512 bytes string]
> >> \.
> COPY 1
>
> Once inserted, I was able to use the pg_replication_origin_xxx
> functions with this origin name without any errors.
> Not sure how common pg_dump/restore of catalog tables is in real
> systems, but should we still consider if this behavior is acceptable?
I'm personally not too worried about this. The limit is primarily there to
provide a nicer ERROR than "row is too big", which AFAICT is the worst-case
scenario if you go to the trouble of setting allow_system_table_mods and
modifying shared catalogs directly.
> 5) As Euler pointed out, logical replication already has a built-in
> restriction for internally assigned origin names, limiting them to
> NAMEDATALEN. Given that, only the SQL function
> pg_replication_origin_create() is capable of creating longer origin
> names. Would it make sense to consider moving the check into
> pg_replication_origin_create() itself, since it seems redundant for
> all other callers?
> That said, if there's a possibility of future callers needing this
> restriction at a lower level, it may be reasonable to leave it as is.
pg_replication_origin_create() might be a better place. After all, that's
where we're enforcing the name doesn't start with "pg_" and, for testing,
_does_ start with "regress_". Plus, it seems highly unlikely that any
other callers of replorigin_create() will ever provide names longer than
512 bytes.
--
nathan
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: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, nisha.moond412@gmail.com, euler@eulerto.com, michael@paquier.xyz, amit.kapila16@gmail.com, exclusion@gmail.com, tgl@sss.pgh.pa.us, jkatz@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
In-Reply-To: <aAfP_bitjVC1bcNV@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox