agora inbox for pgsql-committers@postgresql.org
help / color / mirror / Atom feedFrom: Jeff Davis <pgsql@j-davis.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
To: Jeff Davis <jdavis@postgresql.org>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Perform provider-specific initialization in new functions.
Date: Fri, 06 Mar 2026 12:01:51 -0800
Message-ID: <dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com> (raw)
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>
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".
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.
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
view thread (8+ messages) latest in thread
Message-ID: <dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com>
Permalink: ../dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com/
Also on: postgresql.org/message-id/dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com
reply
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-committers@postgresql.org
Cc: pgsql@j-davis.com, x4mmm@yandex-team.ru, jdavis@postgresql.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Perform provider-specific initialization in new functions.
In-Reply-To: <dba69ec2334f9cff04d102de243ea912a3166586.camel@j-davis.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox