| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Jeff Davis <pgsql(at)j-davis(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Remaining dependency on setlocale() |
| Date: | 2025-12-16 01:32:09 |
| Message-ID: | 651F79AB-8E21-4CDD-9A5A-221AC271265E@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Dec 16, 2025, at 03:34, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> On Sat, 2025-12-13 at 17:48 +0800, Chao Li wrote:
>> 1 - 0001
>> ```
>> + int match_mblen pg_attribute_unused();
>> ```
>>
>> Why this variable is marked unused? It’s actually used.
>
> Fixed and committed.
>
> I originally marked it unused because I just had an:
>
> Assert(match[match_mblen] == '\0');
>
> but I changed it to just set it:
>
> match[match_mblen] = '\0';
>
> for clarity, even though I think the underlying API guarantees NUL-
> termination.
>
>> 2 - 0002
>>
>> I did a search and found one place that you missed at line 181 in
>> pg_locale.h
>> ```
>> extern bool char_is_cased(char ch, pg_locale_t locale);
>> ```
>
> Thank you, fixed and committed.
>
>
> I'll continue committing v12 0003-0005. Note that I'm planning to
> backport 0003.
I have re-reviewed 0003-0005 last week, they all look good to me.
I have no comment on backport 0003.
>
> I'm also inclined to move forward with 0006. It's not a long-term
> solution, but it solves an instance of the problem under discussion in
> this thread (removes setlocale() dependency) and is a narrow fix.
>
>
> After that, remaining loose ends:
>
> * 0007: Can we just rip out the non-ASCII support? If it's gone all
> this time without UTF8 support, and there's no suggestion about what
> the non-ASCII behavior should be (in docs or source code), then it's
> probably not terribly important.
>
> * Use of pg_strncasecmp() in the backend, which uses tolower() to do
> case-insensitive matching of command options, e.g. "if
> (pg_strcasecmp(a, "plain") == 0)" to check for PLAIN storage in CREATE
> TYPE.
>
> * strerror(): either consider an lc_ctype GUC, or the approach over
> here[1].
>
> * Daniel suggests that we need some way to set LC_COLLATE for
> extensions/dependencies.
>
> * address calls to pg_tolower in datetime.c and tzparser.c -- can those
> just be pg_ascii_tolower()?
>
> * examine remaining isalpha(), etc., calls in the backend
>
> Regards,
> Jeff Davis
>
> [1]
> https://www.postgresql.org/message-id/flat/90f176c5b85b9da26a3265b2630ece3552068566(dot)camel(at)j-davis(dot)com
>
I just reviewed 0006-0007. Only got one comment on 0006:
```
@@ -306,6 +342,7 @@ create_pg_locale_icu(Oid collid, MemoryContext context)
result = MemoryContextAllocZero(context, sizeof(struct pg_locale_struct));
result->icu.locale = MemoryContextStrdup(context, iculocstr);
result->icu.ucol = collator;
+ result->icu.lt = loc;
```
The old code didn’t create a locale object and store in result, thus it didn’t have a logic to free the created locale. This patch now dose that, but I don’t see where the created locale object is free-ed. I suppose newlocale() will allocate memory from the OS, so I guess the memory should be free-ed somewhere.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David G. Johnston | 2025-12-16 01:44:33 | Re: doc: create table improvements |
| Previous Message | David G. Johnston | 2025-12-16 01:10:39 | Re: Document NULL |