Re: Crash issue in PG18.5 regression

From: Andres Freund <andres(at)anarazel(dot)de>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, "Masashi Kamura (Fujitsu)" <kamura(dot)masashi(at)fujitsu(dot)com>, "'pgsql-hackers(at)lists(dot)postgresql(dot)org'" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Jeff Davis <pgsql(at)j-davis(dot)com>
Subject: Re: Crash issue in PG18.5 regression
Date: 2026-08-11 18:31:22
Message-ID: v3nniwcrxejmcfvz56xbd22hphprqleuornd6hqkmw2bl7kgmz@cnytz2ee5ltk
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2026-08-11 12:41:49 +0300, Heikki Linnakangas wrote:
> +/* lowercasing/casefolding in C locale */
> +static size_t
> +strlower_c(char *dst, size_t dstsize, const char *src, size_t srclen)
> +{
> + int i;
> +
> + for (i = 0; i < srclen && i < dstsize; i++)
> + dst[i] = pg_ascii_tolower(src[i]);
> + if (i < dstsize)
> + dst[i] = '\0';
> + return srclen;
> +}

Hm. If I infer the pg_strlower() API correctly - it's utterly underdocumented
- it seems to be inteded to support a few things in 18:

1) srclen = -1 works

Inferred from unicode_strlower()'s comment:

* String src must be encoded in UTF-8. If srclen < 0, src must be
* NUL-terminated.

Also note that srclen is ssize_t. This changed in 6d22c67c3bf5 (recently).

2) The required length for the conversion is returned, even if the destination
is too short (including when dstlen = 0)

* Result string is stored in dst, truncating if larger than dstsize. If
* dstsize is greater than the result length, dst will be NUL-terminated;
* otherwise not.
*
* If dstsize is zero, dst may be NULL. This is useful for calculating the
* required buffer size before allocating.

It's really a guessing game though, due to the religious under documentation
of the generic functions. Why does unicode_strlower() have docs, but
pg_strlower() does not?

Both don't seem quite right given this implementation.

I don't quite know whether we need to fix these, given the lack of problematic
uses in tree, the time pressure, but it also seems like a recipe for future
disaster to leave it like this.

I'd also make i size_t, given that the input is size_t. Perhaps practically
no problem, but I see no reason to not use size_t here.

It also seems like we really ought to have an actually reachable, currently
crashing, to_date() call in the tests? It seems concerning that
seq_search_localized(), casefold_str_cmp() are completely uncovered today, and
quite obviously we can't be relied upon to get this right.

https://coverage.postgresql.org/src/backend/utils/adt/formatting.c.gcov.html#L2379

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2026-08-11 18:47:11 Re: [PATCH] bufmgr: tighten LWLock:BufferMapping on InvalidateBuffer
Previous Message Alexandre Felipe 2026-08-11 18:27:37 Re: Restructured Shared Buffer Hash Table