Message-ID: <dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com>
Subject: Re: pgsql: Perform provider-specific initialization in new
 functions.
From: Jeff Davis <pgsql@j-davis.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>, Jeff Davis <jdavis@postgresql.org>
Cc: pgsql-committers@lists.postgresql.org
Date: Fri, 06 Mar 2026 12:01:51 -0800
In-Reply-To: <D18AD72A-5004-4EF8-AF80-10732AF677FA@yandex-team.ru>
References: <E1tINJx-000sMi-Vr@gemulon.postgresql.org>
	 <D18AD72A-5004-4EF8-AF80-10732AF677FA@yandex-team.ru>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.3-0ubuntu1.1 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/dba69ec2334f9cff04d102de243ea912a3166586.camel%40j-davis.com>
Precedence: bulk

On Fri, 2026-03-06 at 20:24 +0500, Andrey Borodin wrote:
> I'm toying with my WAL compression patch. This test is segfaulting
> for me

Thank you for the report!

> wal_compression is PGC_SUSET, so when a non-superuser sets it via the
> startup
> packet (PGC_BACKEND context), set_config_with_handle must call
> pg_parameter_aclcheck -> SearchSysCache1(PARAMETERACLNAME, ...) ->
> hashtext ->
> pg_newlocale_from_collation(DEFAULT_COLLATION_OID).

It seems like the real problem here is in catcache.c:texthashfast(),
which unconditionally passes DEFAULT_COLLATION_OID, despite the fact
that pg_parameter_acl.parname has collation "C".=20

namehashfast() uses C-like semantics, which is OK because all name
columns in the catalog have collation "C". But TEXT columns in the
catalog are about a mix of DEFAULT_COLLATION_OID and C_COLLATION_OID.

There are a few possible fixes:

  1. Your fix addresses it, and would also add some safety against
other edge cases we haven't caught yet. The only time it would take
effect is for very early initialization, but there is nonzero risk of
inconsistency because the same value would get a different hash before
and after CheckMyDatabase().

  2. We could hardcode texthashfunc() to use C_COLLATION_OID. That
wouldn't match the column collation, but it would avoid the crash, and
might technically still be fine: the default collation is always
deterministic, and all deterministic collations have the same equality
semantics as "C". Even if the proper hashtext() is used somewhere else,
then it uses "C" hashing semantics for all deterministic collations.
The problem here is that we'd like to allow the default collation to be
nondeterministic in the future (Peter has mentioned this a few times),
so relying on this assumption is fragile.=20

  3. We could try to include collation information in the cachinfo or
somewhere and pass it down to find the right hash function. This feels
like a better fix, but there could be other areas we miss that are
using a catalog TEXT field with the default collation. Also it's more
invasive.

We could decide to do your approach for now (in master and
REL_18_STABLE), and then leave #3 for the future (master only).

Thoughts?

Regards,
	Jeff Davis