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
29 matches
Mail list logo
