On 12.09.24 22:01, Jeff Davis wrote:
On Mon, 2024-09-09 at 15:37 +0200, Peter Eisentraut wrote:
In the end, I figured the best thing to do here is to add an explicit
locale argument to MATCH_LOWER() and GETCHAR() so one can actually
understand this code by reading it. New patch attached.
Look
On 9/4/24 11:45 PM, Jeff Davis wrote:
Committed v2-0001.
> [...]
I fixed this by replacing the assert with an elog(ERROR, ...), so that
it will consistently show a "cache lookup failed for collation 0"
regardless of whether it's a debug build or not. It's not expected that
the error will be e
On Mon, 2024-09-09 at 15:37 +0200, Peter Eisentraut wrote:
> In the end, I figured the best thing to do here is to add an explicit
> locale argument to MATCH_LOWER() and GETCHAR() so one can actually
> understand this code by reading it. New patch attached.
Looks good to me, thank you.
Regards,
On 08.08.24 22:00, Jeff Davis wrote:
On Wed, 2024-08-07 at 22:44 +0200, Peter Eisentraut wrote:
But after this patch set, locale cannot be NULL anymore, so the third
branch is obsolete.
...
Second, there are a number of functions in like.c like the above that
take separate arguments like pg_
Committed v2-0001.
On Tue, 2024-09-03 at 22:04 -0700, Jeff Davis wrote:
> * This patch may change the handling of collation oid 0, and I'm not
> sure whether that was intentional or not. lc_collate_is_c(0) returned
> false, whereas pg_newlocale_from_collation(0)->collate_is_c raised
> Assert or t
On Wed, 2024-08-28 at 18:43 +0200, Andreas Karlsson wrote:
> On 8/15/24 12:55 AM, Jeff Davis wrote:
> > This overlaps a bit with what Peter already proposed here:
> >
> > https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f%40eisentraut.org
> >
> > right?
>
> Maybe I am mis
On 8/15/24 12:55 AM, Jeff Davis wrote:
This overlaps a bit with what Peter already proposed here:
https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f%40eisentraut.org
right?
Maybe I am missing something but not as far as I can see. It is related
but I do not see any ov
On Wed, 2024-08-14 at 01:31 +0200, Andreas Karlsson wrote:
> 0001-Remove-lc_collate_is_c.patch
>
> Removes lc_collate_is_c().
This could use some performance testing, as the commit message says,
otherwise it looks good.
> 0002-Remove-lc_ctype_is_c.patch
>
> Removes lc_ctype_is_c() and POSIX_COL
On 8/13/24 7:56 PM, Jeff Davis wrote:
But I don't think that's a major problem -- we can just move the
hardcoded test into pg_newlocale_from_collation() and return a
predefined struct with collate_is_c/ctype_is_c already set.
I tried that out but thought it felt cleaner to do the hardcoding in
On Tue, 2024-08-13 at 17:11 +0200, Peter Eisentraut wrote:
> They used to be completely separate from
> pg_newlocale_from_collation(),
> but now they are just mostly a thin wrapper around it. Except there
> is
> some hardcoded handling of C_COLLATION_OID and POSIX_COLLATION_OID.
> Do
> we car
I'm wondering if after this patch series, lc_collate_is_c() and
lc_ctype_is_c() are still useful.
They used to be completely separate from pg_newlocale_from_collation(),
but now they are just mostly a thin wrapper around it. Except there is
some hardcoded handling of C_COLLATION_OID and POSIX
On Mon, 2024-08-12 at 09:00 +0200, Peter Eisentraut wrote:
> > I assume it would now be okay to take out "locale &&" here?
>
> Correct, that check is no longer necessary.
Thank you, fixed.
Regards,
Jeff Davis
On 11.08.24 18:33, Tom Lane wrote:
Jeff Davis writes:
We can address those as part of a separate thread. I'll count this as
committed.
Coverity has a nit about this:
*** CID 1616189: Null pointer dereferences (REVERSE_INULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/utils/adt/like
Jeff Davis writes:
> We can address those as part of a separate thread. I'll count this as
> committed.
Coverity has a nit about this:
*** CID 1616189: Null pointer dereferences (REVERSE_INULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/utils/adt/like.c: 206 in
Generic_Text_IC_like()
On Thu, 2024-08-08 at 23:09 +0200, Peter Eisentraut wrote:
> With current master, after commit e9931bfb751, you get the first
> result
> in both cases.
>
> So this could break indexes across pg_upgrade in such configurations.
Good observation. Is there any action we should take here, or just add
On 08.08.24 22:00, Jeff Davis wrote:
On Wed, 2024-08-07 at 22:44 +0200, Peter Eisentraut wrote:
But after this patch set, locale cannot be NULL anymore, so the third
branch is obsolete.
...
Second, there are a number of functions in like.c like the above that
take separate arguments like pg_
On 07.08.24 22:44, Peter Eisentraut wrote:
(Now that I look at it, pg_tolower() has some short-circuiting for ASCII
letters, so it would not handle Turkish-i correctly if that had been the
global locale. By removing the use of pg_tolower(), we fix that issue
in passing.)
It occurred to me th
On Wed, 2024-08-07 at 22:44 +0200, Peter Eisentraut wrote:
> But after this patch set, locale cannot be NULL anymore, so the third
> branch is obsolete.
...
> Second, there are a number of functions in like.c like the above that
> take separate arguments like pg_locale_t locale, bool locale_is_c.
On 06.08.24 23:40, Jeff Davis wrote:
With these changes, collations are no longer dependent on the
environment locale (setlocale()) at all for either collation behavior
(ORDER BY) or ctype behavior (LOWER(), etc.).
Additionally, unless I missed something, nothing in the server is
dependent on LC
On Tue, 2024-07-30 at 12:13 -0700, Jeff Davis wrote:
> I found a couple issues with the later patches:
>
> * There was still some confusion about the default collation vs.
> datcollate/datctype for callers of wchar2char() and char2wchar()
> (those
> functions only work for libc). I introduced a ne
On Mon, 2024-07-29 at 21:45 +0200, Peter Eisentraut wrote:
> I have also re-reviewed the patches and I agree they are good to go.
I found a couple issues with the later patches:
* There was still some confusion about the default collation vs.
datcollate/datctype for callers of wchar2char() and ch
On 27.07.24 21:03, Andreas Karlsson wrote:
On 7/26/24 10:35 PM, Jeff Davis wrote:
database_ctype_is_c refers to the LC_CTYPE environment of the database
-- pg_database.datctype. default_locale.ctype_is_c is the ctype of the
database's default collation.
Confusing, I know, but it matters for a f
On 7/26/24 10:35 PM, Jeff Davis wrote:
database_ctype_is_c refers to the LC_CTYPE environment of the database
-- pg_database.datctype. default_locale.ctype_is_c is the ctype of the
database's default collation.
Confusing, I know, but it matters for a few things that still depend on
the LC_CTYPE,
On Fri, 2024-07-26 at 19:38 +0200, Andreas Karlsson wrote:
> Nice refactoring!
>
> Two small comments about CheckMyDatabase().
>
> - Shouldn't we look at the default_locale.ctype_is_c when setting
> database_ctype_is_c instead of doing a strcmp()? or maybe we should
> even
> remove the global v
Nice refactoring!
Two small comments about CheckMyDatabase().
- Shouldn't we look at the default_locale.ctype_is_c when setting
database_ctype_is_c instead of doing a strcmp()? or maybe we should even
remove the global variable and always look at the default_locale?
- I think that the lookup
On Wed, 2024-06-19 at 11:15 +0200, Peter Eisentraut wrote:
> > * v2-0001-Make-database-default-collation-internal-to-pg_lo.patch
> >
> > +void
> > +pg_init_database_collation()
> >
> > The function argument should be (void).
> >
> > The prefix pg_ makes it sound like it's a user-facing function
On 15.06.24 01:35, Jeff Davis wrote:
On Thu, 2024-06-06 at 11:37 -0700, Jeff Davis wrote:
I think this patch series is a nice cleanup, as well, making libc
more
like the other providers and not dependent on global state.
New rebased series attached with additional cleanup. Now that
pg_locale_
On Thu, 2024-06-06 at 11:37 -0700, Jeff Davis wrote:
>
> I think this patch series is a nice cleanup, as well, making libc
> more
> like the other providers and not dependent on global state.
New rebased series attached with additional cleanup. Now that
pg_locale_t is never NULL, we can simplify
On Wed, 2024-06-05 at 17:23 -0700, Jeff Davis wrote:
> A brief test shows that there may be a performance regression for
> libc
> default collations. But if so, I'm not sure that's avoidable if the
> goal is to take away setlocale. I'll see if removing the extra
> branches
> mitigates it.
I redid
There was an unconference session at pgconf.dev related to threading
support. One of the problems identified was setlocale().
The attached series of patches make collation not depend on
setlocale(), even if the database collation uses the libc provider.
Since commit 8d9a9f034e, all supported pla
30 matches
Mail list logo
