public inbox for [email protected]
help / color / mirror / Atom feedRe: Built-in CTYPE provider
14+ messages / 5 participants
[nested] [flat]
* Re: Built-in CTYPE provider
@ 2024-07-02 16:51 Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2024-07-02 16:51 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Mon, Jul 01, 2024 at 06:19:08PM -0700, Jeff Davis wrote:
> On Mon, 2024-07-01 at 16:03 -0700, Noah Misch wrote:
> > An alternative would be to make pg_upgrade reject
> > operating on a cluster that contains use of $SUBJECT.
>
> That wouldn't help anyone.
Can you say more about that? For the last decade at least, I think our
standard for new features has been to error rather than allow an operation
that creates a known path to wrong query results. I think that's a helpful
standard that we should continue to follow.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-02 22:05 ` Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Peter Eisentraut @ 2024-07-02 22:05 UTC (permalink / raw)
To: Noah Misch <[email protected]>; Jeff Davis <[email protected]>; +Cc: Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On 02.07.24 18:51, Noah Misch wrote:
> On Mon, Jul 01, 2024 at 06:19:08PM -0700, Jeff Davis wrote:
>> On Mon, 2024-07-01 at 16:03 -0700, Noah Misch wrote:
>>> An alternative would be to make pg_upgrade reject
>>> operating on a cluster that contains use of $SUBJECT.
>>
>> That wouldn't help anyone.
>
> Can you say more about that? For the last decade at least, I think our
> standard for new features has been to error rather than allow an operation
> that creates a known path to wrong query results. I think that's a helpful
> standard that we should continue to follow.
I don't think the builtin locale provider is any different in this
respect from the other providers: The locale data might change and
there is a version mechanism to track that. We don't prevent pg_upgrade
in scenarios like that for other providers.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
@ 2024-07-02 23:03 ` Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2024-07-02 23:03 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jeff Davis <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Wed, Jul 03, 2024 at 12:05:09AM +0200, Peter Eisentraut wrote:
> On 02.07.24 18:51, Noah Misch wrote:
> > On Mon, Jul 01, 2024 at 06:19:08PM -0700, Jeff Davis wrote:
> > > On Mon, 2024-07-01 at 16:03 -0700, Noah Misch wrote:
> > > > An alternative would be to make pg_upgrade reject
> > > > operating on a cluster that contains use of $SUBJECT.
> > >
> > > That wouldn't help anyone.
> >
> > Can you say more about that? For the last decade at least, I think our
> > standard for new features has been to error rather than allow an operation
> > that creates a known path to wrong query results. I think that's a helpful
> > standard that we should continue to follow.
>
> I don't think the builtin locale provider is any different in this respect
> from the other providers: The locale data might change and there is a
> version mechanism to track that. We don't prevent pg_upgrade in scenarios
> like that for other providers.
Each packager can choose their dependencies so the v16 providers don't have
the problem. With the $SUBJECT provider, a packager won't have that option.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-03 21:19 ` Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Davis @ 2024-07-03 21:19 UTC (permalink / raw)
To: Noah Misch <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Tue, 2024-07-02 at 16:03 -0700, Noah Misch wrote:
> Each packager can choose their dependencies so the v16 providers
> don't have
> the problem. With the $SUBJECT provider, a packager won't have that
> option.
While nothing needs to be changed for 17, I agree that we may need to
be careful in future releases not to break things.
Broadly speaking, you are right that we may need to freeze Unicode
updates or be more precise about versioning. But there's a lot of
nuance to the problem, so I don't think we should pre-emptively promise
either of those things right now.
Consider:
* Unless I made a mistake, the last three releases of Unicode (14.0,
15.0, and 15.1) all have the exact same behavior for UPPER() and
LOWER() -- even for unassigned code points. It would be silly to
promise to stay with 15.1 and then realize that moving to 16.0 doesn't
create any actual problem.
* Unicode also offers "case folding", which has even stronger stability
guarantees, and I plan to propose that soon. When implemented, it would
be preferred over LOWER()/UPPER() in index expressions for most use
cases.
* While someone can pin libc+ICU to particular versions, it's
impossible when using the official packages, and additionally requires
using something like [1], which just became available last year. I
don't think it's reasonable to put it forth as a matter-of-fact
solution.
* Let's keep some perspective: we've lived for a long time with ALL
text indexes at serious risk of breakage. In contrast, the concerns you
are raising now are about certain kinds of expression indexes over data
containing certain unassigned code points. I am not dismissing that
concern, but the builtin provider moves us in the right direction and
let's not lose sight of that.
Given that no code changes for v17 are proposed, I suggest that we
refrain from making any declarations until the next version of Unicode
is released. If the pattern holds, that will be around September, which
still leaves time to make reasonable decisions for v18.
Regards,
Jeff Davis
[1] https://github.com/awslabs/compat-collation-for-glibc
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
@ 2024-07-04 21:26 ` Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2024-07-04 21:26 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Wed, Jul 03, 2024 at 02:19:07PM -0700, Jeff Davis wrote:
> * Unless I made a mistake, the last three releases of Unicode (14.0,
> 15.0, and 15.1) all have the exact same behavior for UPPER() and
> LOWER() -- even for unassigned code points. It would be silly to
> promise to stay with 15.1 and then realize that moving to 16.0 doesn't
> create any actual problem.
I think you're saying that if some Unicode update changes the results of a
STABLE function but does not change the result of any IMMUTABLE function, we
may as well import that update. Is that about right? If so, I agree.
In addition to the options I listed earlier (error in pg_upgrade or document
that IMMUTABLE stands) I would be okay with a third option. Decide here that
we'll not adopt a Unicode update in a way that changes a v17 IMMUTABLE
function result of the new provider. We don't need to write that in the
documentation, since it's implicit in IMMUTABLE. Delete the "stable within a
<productname>Postgres</productname> major version" documentation text.
> * While someone can pin libc+ICU to particular versions, it's
> impossible when using the official packages, and additionally requires
> using something like [1], which just became available last year. I
> don't think it's reasonable to put it forth as a matter-of-fact
> solution.
>
> * Let's keep some perspective: we've lived for a long time with ALL
> text indexes at serious risk of breakage. In contrast, the concerns you
> are raising now are about certain kinds of expression indexes over data
> containing certain unassigned code points. I am not dismissing that
> concern, but the builtin provider moves us in the right direction and
> let's not lose sight of that.
I see you're trying to help users get less breakage, and that's a good goal.
I agree $SUBJECT eliminates libc+ICU breakage, and libc+ICU breakage has hurt
plenty. However, you proposed to update Unicode data and give REINDEX as the
solution to breakage this causes. Unlike libc+ICU breakage, the packager has
no escape from that. That's a different kind of breakage proposition, and no
new PostgreSQL feature should do that. It's on a different axis from helping
users avoid libc+ICU breakage, and a feature doesn't get to credit helping on
one axis against a regression on the other axis. What am I missing here?
> Given that no code changes for v17 are proposed, I suggest that we
> refrain from making any declarations until the next version of Unicode
> is released. If the pattern holds, that will be around September, which
> still leaves time to make reasonable decisions for v18.
Soon enough, a Unicode release will add one character to regexp [[:alpha:]].
PostgreSQL will then need to decide what IMMUTABLE is going to mean. How does
that get easier in September?
Thanks,
nm
> [1] https://github.com/awslabs/compat-collation-for-glibc
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-05 21:38 ` Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Davis @ 2024-07-05 21:38 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Thu, 2024-07-04 at 14:26 -0700, Noah Misch wrote:
> I think you're saying that if some Unicode update changes the results
> of a
> STABLE function but does not change the result of any IMMUTABLE
> function, we
> may as well import that update. Is that about right? If so, I
> agree.
If you are proposing that Unicode updates should not be performed if
they affect the results of any IMMUTABLE function, then that's a new
policy.
For instance, the results of NORMALIZE() changed from PG15 to PG16 due
to commit 1091b48cd7:
SELECT NORMALIZE(U&'\+01E030',nfkc)::bytea;
Version 15: \xf09e80b0
Version 16: \xd0b0
I am neither endorsing nor opposing the new policy you propose at this
time, but deep in the sub-thread of one particular feature is not the
right place to discuss it.
Please start a new thread for the proposed PG18 policy change and CC
me. I happen to think that around the release of the next version of
Unicode (in a couple months) would be the most productive time to have
that discussion, but you can start the discussion now if you like.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
@ 2024-07-06 19:51 ` Noah Misch <[email protected]>
2024-07-06 20:19 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2024-07-06 19:51 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Fri, Jul 05, 2024 at 02:38:45PM -0700, Jeff Davis wrote:
> On Thu, 2024-07-04 at 14:26 -0700, Noah Misch wrote:
> > I think you're saying that if some Unicode update changes the results
> > of a
> > STABLE function but does not change the result of any IMMUTABLE
> > function, we
> > may as well import that update. Is that about right? If so, I
> > agree.
>
> If you are proposing that Unicode updates should not be performed if
> they affect the results of any IMMUTABLE function, then that's a new
> policy.
>
> For instance, the results of NORMALIZE() changed from PG15 to PG16 due
> to commit 1091b48cd7:
>
> SELECT NORMALIZE(U&'\+01E030',nfkc)::bytea;
>
> Version 15: \xf09e80b0
>
> Version 16: \xd0b0
As a released feature, NORMALIZE() has a different set of remedies to choose
from, and I'm not proposing one. I may have sidetracked this thread by
talking about remedies without an agreement that pg_c_utf8 has a problem. My
question for the PostgreSQL maintainers is this:
textregexeq(... COLLATE pg_c_utf8, '[[:alpha:]]') and lower(), despite being
IMMUTABLE, will change behavior in some major releases. pg_upgrade does not
have a concept of IMMUTABLE functions changing, so index scans will return
wrong query results after upgrade. Is it okay for v17 to release a
pg_c_utf8 planned to behave that way when upgrading v17 to v18+?
If the answer is yes, the open item closes. If the answer is no, determining
the remedy can come next.
Lest concrete details help anyone reading, here are some affected objects:
CREATE TABLE t (s text COLLATE pg_c_utf8);
INSERT INTO t VALUES (U&'\+00a7dc'), (U&'\+001dd3');
CREATE INDEX iexpr ON t ((lower(s)));
CREATE INDEX ipred ON t (s) WHERE s ~ '[[:alpha:]]';
v17 can simulate the Unicode aspect of a v18 upgrade, like this:
sed -i 's/^UNICODE_VERSION.*/UNICODE_VERSION = 16.0.0/' src/Makefile.global.in
# ignore test failures (your ICU likely doesn't have the Unicode 16.0.0 draft)
make -C src/common/unicode update-unicode
make
make install
pg_ctl restart
Behavior after that:
-- 2 rows w/ seq scan, 0 rows w/ index scan
SELECT 1 FROM t WHERE s ~ '[[:alpha:]]';
SET enable_seqscan = off;
SELECT 1 FROM t WHERE s ~ '[[:alpha:]]';
-- ERROR: heap tuple (0,1) from table "t" lacks matching index tuple within index "iexpr"
SELECT bt_index_parent_check('iexpr', heapallindexed => true);
-- ERROR: heap tuple (0,1) from table "t" lacks matching index tuple within index "ipred"
SELECT bt_index_parent_check('ipred', heapallindexed => true);
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-06 20:19 ` Tom Lane <[email protected]>
2024-07-09 01:05 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2024-07-06 20:19 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Jeff Davis <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
Noah Misch <[email protected]> writes:
> As a released feature, NORMALIZE() has a different set of remedies to choose
> from, and I'm not proposing one. I may have sidetracked this thread by
> talking about remedies without an agreement that pg_c_utf8 has a problem. My
> question for the PostgreSQL maintainers is this:
> textregexeq(... COLLATE pg_c_utf8, '[[:alpha:]]') and lower(), despite being
> IMMUTABLE, will change behavior in some major releases. pg_upgrade does not
> have a concept of IMMUTABLE functions changing, so index scans will return
> wrong query results after upgrade. Is it okay for v17 to release a
> pg_c_utf8 planned to behave that way when upgrading v17 to v18+?
I do not think it is realistic to define "IMMUTABLE" as meaning that
the function will never change behavior until the heat death of the
universe. As a counterexample, we've not worried about applying
bug fixes or algorithm improvements that change the behavior of
"immutable" numeric computations. It might be unwise to do that
in a minor release, but we certainly do it in major releases.
I'd say a realistic policy is "immutable means we don't intend to
change it within a major release". If we do change the behavior,
either as a bug fix or a major-release improvement, that should
be release-noted so that people know they have to rebuild dependent
indexes and matviews.
It gets stickier for behaviors that aren't fully under our control,
which is the case for a lot of locale-related things. We cannot then
promise "no changes within major releases". But I do not think it
is helpful to react to that fact by refusing to label such things
immutable. Then we'd just need another mutability classification,
and it would effectively act the same as immutable does now, because
people will certainly wish to use these functions in indexes etc.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-06 20:19 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
@ 2024-07-09 01:05 ` Noah Misch <[email protected]>
2024-07-09 01:17 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
2024-07-09 23:20 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Noah Misch @ 2024-07-09 01:05 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jeff Davis <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Sat, Jul 06, 2024 at 04:19:21PM -0400, Tom Lane wrote:
> Noah Misch <[email protected]> writes:
> > As a released feature, NORMALIZE() has a different set of remedies to choose
> > from, and I'm not proposing one. I may have sidetracked this thread by
> > talking about remedies without an agreement that pg_c_utf8 has a problem. My
> > question for the PostgreSQL maintainers is this:
>
> > textregexeq(... COLLATE pg_c_utf8, '[[:alpha:]]') and lower(), despite being
> > IMMUTABLE, will change behavior in some major releases. pg_upgrade does not
> > have a concept of IMMUTABLE functions changing, so index scans will return
> > wrong query results after upgrade. Is it okay for v17 to release a
> > pg_c_utf8 planned to behave that way when upgrading v17 to v18+?
>
> I do not think it is realistic to define "IMMUTABLE" as meaning that
> the function will never change behavior until the heat death of the
> universe. As a counterexample, we've not worried about applying
> bug fixes or algorithm improvements that change the behavior of
> "immutable" numeric computations.
True. There's a continuum from "releases can change any IMMUTABLE function"
to "index integrity always wins, even if a function is as wrong as 1+1=3".
I'm less concerned about the recent "Incorrect results from numeric round"
thread, even though it's proposing to back-patch. I'm thinking about these
aggravating factors for $SUBJECT:
- $SUBJECT is planning an annual cadence of this kind of change.
- We already have ICU providing collation support for the same functions.
Unlike $SUBJECT, ICU integration gives packagers control over when to accept
corruption at pg_upgrade time.
- SQL Server, DB2 and Oracle do their Unicode updates in a non-corrupting way.
(See Jeremy Schneider's reply concerning DB2 and Oracle.)
- lower() and regexp are more popular in index expressions than
high-digit-count numeric calculations.
> I'd say a realistic policy is "immutable means we don't intend to
> change it within a major release". If we do change the behavior,
> either as a bug fix or a major-release improvement, that should
> be release-noted so that people know they have to rebuild dependent
> indexes and matviews.
It sounds like you're very comfortable with $SUBJECT proceeding in its current
form. Is that right?
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-06 20:19 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
2024-07-09 01:05 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-09 01:17 ` Tom Lane <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Tom Lane @ 2024-07-09 01:17 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Jeff Davis <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
Noah Misch <[email protected]> writes:
> It sounds like you're very comfortable with $SUBJECT proceeding in its current
> form. Is that right?
I don't have an opinion on whether the overall feature design
is well-chosen. But the mere fact that Unicode updates will
from time to time change the behavior (presumably only in edge
cases or for previously-unassigned code points) doesn't strike
me as a big enough problem to justify saying these functions
can't be marked immutable anymore. Especially since we have been
faced with that problem all along anyway; we just didn't have a way
to track or quantify it before, because locale changes happened
outside code we control.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-06 20:19 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
2024-07-09 01:05 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
@ 2024-07-09 23:20 ` Jeff Davis <[email protected]>
2024-07-11 12:50 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Jeff Davis @ 2024-07-09 23:20 UTC (permalink / raw)
To: Noah Misch <[email protected]>; Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Mon, 2024-07-08 at 18:05 -0700, Noah Misch wrote:
> I'm thinking about these
> aggravating factors for $SUBJECT:
This is still marked as an open item for 17, but you've already
acknowledged[1] that no code changes are necessary in version 17.
Upgrades of Unicode take an active step from a committer, so it's not a
pressing problem for 18, either.
The idea that you're arguing against is "stability within a PG major
version". There's no new discovery here: it was listed under the
heading of "Benefits" near the top of my initial proposal[2], and known
to all reviewers.
This is not an Open Item for 17, and new policy discussions should not
happen deep in this subthread. Please resolve the Open Item, and feel
free to start a thread about policy changes in 18.
Regards,
Jeff Davis
[1]
https://www.postgresql.org/message-id/[email protected]
[2]
https://www.postgresql.org/message-id/[email protected]
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Built-in CTYPE provider
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Re: Built-in CTYPE provider Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-03 21:19 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-04 21:26 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-05 21:38 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
2024-07-06 19:51 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-06 20:19 ` Re: Built-in CTYPE provider Tom Lane <[email protected]>
2024-07-09 01:05 ` Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-09 23:20 ` Re: Built-in CTYPE provider Jeff Davis <[email protected]>
@ 2024-07-11 12:50 ` Noah Misch <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Noah Misch @ 2024-07-11 12:50 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Verite <[email protected]>; Robert Haas <[email protected]>; Jeremy Schneider <[email protected]>; pgsql-hackers
On Tue, Jul 09, 2024 at 04:20:12PM -0700, Jeff Davis wrote:
> On Mon, 2024-07-08 at 18:05 -0700, Noah Misch wrote:
> > I'm thinking about these
> > aggravating factors for $SUBJECT:
>
> This is still marked as an open item for 17, but you've already
> acknowledged[1] that no code changes are necessary in version 17.
Later posts on the thread made that obsolete. The next step is to settle the
question at https://postgr.es/m/[email protected]. If that
conclusion entails a remedy, v17 code changes may be part of that remedy.
> The idea that you're arguing against is "stability within a PG major
> version". There's no new discovery here: it was listed under the
> heading of "Benefits" near the top of my initial proposal[2]
Thanks for that distillation. More specifically, I'm arguing against the lack
of choice about instability across major versions:
| ICU collations | pg_c_utf8
----------------------------------|-------------------|----------
Corruption within a major version | packager's choice | no
Corruption at pg_upgrade time | packager's choice | yes
I am a packager who chooses no-corruption (chooses stability). As a packager,
the pg_c_utf8 stability within major versions is never a bad thing, but it
does not compensate for instability across major versions. I don't want a
future in which pg_c_utf8 is the one provider that integrity-demanding
pg_upgrade users should not use.
> and known to all reviewers.
If after https://postgr.es/m/[email protected] and
https://postgr.es/m/[email protected] they think $SUBJECT
should continue as-committed, they should vote that way. Currently, we have
multiple votes in one direction and multiple votes in the other direction. If
all three reviewers were to vote in the same direction (assuming no other new
votes), I argue that such a count would render whichever way they vote as the
conclusion. Does that match your count?
> [1]
> https://www.postgresql.org/message-id/[email protected]
> [2]
> https://www.postgresql.org/message-id/[email protected]
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v1 6/8] convert SharedFileSet->refcnt to an atomic
@ 2026-07-09 19:53 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Nathan Bossart @ 2026-07-09 19:53 UTC (permalink / raw)
---
src/backend/storage/file/sharedfileset.c | 27 +++++++++---------------
src/include/storage/sharedfileset.h | 5 ++---
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/backend/storage/file/sharedfileset.c b/src/backend/storage/file/sharedfileset.c
index d76bd72dc63..4f12f92beae 100644
--- a/src/backend/storage/file/sharedfileset.c
+++ b/src/backend/storage/file/sharedfileset.c
@@ -38,8 +38,7 @@ void
SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg)
{
/* Initialize the shared fileset specific members. */
- SpinLockInit(&fileset->mutex);
- fileset->refcnt = 1;
+ pg_atomic_init_u32(&fileset->refcnt, 1);
/* Initialize the fileset. */
FileSetInit(&fileset->fs);
@@ -55,19 +54,15 @@ SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg)
void
SharedFileSetAttach(SharedFileSet *fileset, dsm_segment *seg)
{
- bool success;
+ uint32 refcnt;
- SpinLockAcquire(&fileset->mutex);
- if (fileset->refcnt == 0)
- success = false;
- else
- {
- ++fileset->refcnt;
- success = true;
- }
- SpinLockRelease(&fileset->mutex);
+ refcnt = pg_atomic_read_u32(&fileset->refcnt);
+ while (refcnt != 0 &&
+ !pg_atomic_compare_exchange_u32(&fileset->refcnt, &refcnt,
+ refcnt + 1))
+ ;
- if (!success)
+ if (refcnt == 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("could not attach to a SharedFileSet that is already destroyed")));
@@ -98,11 +93,9 @@ SharedFileSetOnDetach(dsm_segment *segment, Datum datum)
bool unlink_all = false;
SharedFileSet *fileset = (SharedFileSet *) DatumGetPointer(datum);
- SpinLockAcquire(&fileset->mutex);
- Assert(fileset->refcnt > 0);
- if (--fileset->refcnt == 0)
+ Assert(pg_atomic_read_u32(&fileset->refcnt) > 0);
+ if (pg_atomic_sub_fetch_u32(&fileset->refcnt, 1) == 0)
unlink_all = true;
- SpinLockRelease(&fileset->mutex);
/*
* If we are the last to detach, we delete the directory in all
diff --git a/src/include/storage/sharedfileset.h b/src/include/storage/sharedfileset.h
index 904396e7173..d89626ae64b 100644
--- a/src/include/storage/sharedfileset.h
+++ b/src/include/storage/sharedfileset.h
@@ -15,10 +15,10 @@
#ifndef SHAREDFILESET_H
#define SHAREDFILESET_H
+#include "port/atomics.h"
#include "storage/dsm.h"
#include "storage/fd.h"
#include "storage/fileset.h"
-#include "storage/spin.h"
/*
* A set of temporary files that can be shared by multiple backends.
@@ -26,8 +26,7 @@
typedef struct SharedFileSet
{
FileSet fs;
- slock_t mutex; /* mutex protecting the reference count */
- int refcnt; /* number of attached backends */
+ pg_atomic_uint32 refcnt; /* number of attached backends */
} SharedFileSet;
extern void SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg);
--
2.50.1 (Apple Git-155)
--Ot5x3ffkWEuxilWO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v1-0007-convert-ParallelBlockTableScanDescData-phs_-start.patch
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v1 6/8] convert SharedFileSet->refcnt to an atomic
@ 2026-07-09 19:53 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Nathan Bossart @ 2026-07-09 19:53 UTC (permalink / raw)
---
src/backend/storage/file/sharedfileset.c | 27 +++++++++---------------
src/include/storage/sharedfileset.h | 5 ++---
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/backend/storage/file/sharedfileset.c b/src/backend/storage/file/sharedfileset.c
index d76bd72dc63..4f12f92beae 100644
--- a/src/backend/storage/file/sharedfileset.c
+++ b/src/backend/storage/file/sharedfileset.c
@@ -38,8 +38,7 @@ void
SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg)
{
/* Initialize the shared fileset specific members. */
- SpinLockInit(&fileset->mutex);
- fileset->refcnt = 1;
+ pg_atomic_init_u32(&fileset->refcnt, 1);
/* Initialize the fileset. */
FileSetInit(&fileset->fs);
@@ -55,19 +54,15 @@ SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg)
void
SharedFileSetAttach(SharedFileSet *fileset, dsm_segment *seg)
{
- bool success;
+ uint32 refcnt;
- SpinLockAcquire(&fileset->mutex);
- if (fileset->refcnt == 0)
- success = false;
- else
- {
- ++fileset->refcnt;
- success = true;
- }
- SpinLockRelease(&fileset->mutex);
+ refcnt = pg_atomic_read_u32(&fileset->refcnt);
+ while (refcnt != 0 &&
+ !pg_atomic_compare_exchange_u32(&fileset->refcnt, &refcnt,
+ refcnt + 1))
+ ;
- if (!success)
+ if (refcnt == 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("could not attach to a SharedFileSet that is already destroyed")));
@@ -98,11 +93,9 @@ SharedFileSetOnDetach(dsm_segment *segment, Datum datum)
bool unlink_all = false;
SharedFileSet *fileset = (SharedFileSet *) DatumGetPointer(datum);
- SpinLockAcquire(&fileset->mutex);
- Assert(fileset->refcnt > 0);
- if (--fileset->refcnt == 0)
+ Assert(pg_atomic_read_u32(&fileset->refcnt) > 0);
+ if (pg_atomic_sub_fetch_u32(&fileset->refcnt, 1) == 0)
unlink_all = true;
- SpinLockRelease(&fileset->mutex);
/*
* If we are the last to detach, we delete the directory in all
diff --git a/src/include/storage/sharedfileset.h b/src/include/storage/sharedfileset.h
index 904396e7173..d89626ae64b 100644
--- a/src/include/storage/sharedfileset.h
+++ b/src/include/storage/sharedfileset.h
@@ -15,10 +15,10 @@
#ifndef SHAREDFILESET_H
#define SHAREDFILESET_H
+#include "port/atomics.h"
#include "storage/dsm.h"
#include "storage/fd.h"
#include "storage/fileset.h"
-#include "storage/spin.h"
/*
* A set of temporary files that can be shared by multiple backends.
@@ -26,8 +26,7 @@
typedef struct SharedFileSet
{
FileSet fs;
- slock_t mutex; /* mutex protecting the reference count */
- int refcnt; /* number of attached backends */
+ pg_atomic_uint32 refcnt; /* number of attached backends */
} SharedFileSet;
extern void SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg);
--
2.50.1 (Apple Git-155)
--Ot5x3ffkWEuxilWO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v1-0007-convert-ParallelBlockTableScanDescData-phs_-start.patch
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-07-09 19:53 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-07-02 16:51 Re: Built-in CTYPE provider Noah Misch <[email protected]>
2024-07-02 22:05 ` Peter Eisentraut <[email protected]>
2024-07-02 23:03 ` Noah Misch <[email protected]>
2024-07-03 21:19 ` Jeff Davis <[email protected]>
2024-07-04 21:26 ` Noah Misch <[email protected]>
2024-07-05 21:38 ` Jeff Davis <[email protected]>
2024-07-06 19:51 ` Noah Misch <[email protected]>
2024-07-06 20:19 ` Tom Lane <[email protected]>
2024-07-09 01:05 ` Noah Misch <[email protected]>
2024-07-09 01:17 ` Tom Lane <[email protected]>
2024-07-09 23:20 ` Jeff Davis <[email protected]>
2024-07-11 12:50 ` Noah Misch <[email protected]>
2026-07-09 19:53 [PATCH v1 6/8] convert SharedFileSet->refcnt to an atomic Nathan Bossart <[email protected]>
2026-07-09 19:53 [PATCH v1 6/8] convert SharedFileSet->refcnt to an atomic Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox