| From: | "Tristan Partin" <tristan(at)partin(dot)io> |
|---|---|
| To: | "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Rui Zhao" <zhaorui126(at)gmail(dot)com> |
| Cc: | "Ayush Tiwari" <ayushtiwari(dot)slg01(at)gmail(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add ASCII fast path to Unicode normalization functions |
| Date: | 2026-10-06 21:56:47 |
| Message-ID: | DLY3AKOXN6GG.VUSDY8KL3MN5@partin.io |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue Oct 6, 2026 at 9:22 PM UTC, Andrew Dunstan wrote:
>
> On 2026-10-04 Su 12:42 PM, Rui Zhao wrote:
>> Hi Andrew,
>>
>> Thanks for working on this. I tried two further optimizations on top
>> of v4. They can be applied independently:
>>
>> 1. unicode_assigned() still calls pg_mbstrlen_with_len() on the
>> remaining input, then walks it again to check each code point. 0001
>> uses the byte length to walk to the end directly. This also avoids
>> scanning the whole suffix before returning false for an unassigned
>> code point near the beginning.
>>
>> 2. unicode_normalize_func() and unicode_is_normalized() allocate a C
>> string for the normalization form on every call, even when the input
>> is all ASCII. 0002 compares the form directly in the text value,
>> preserving case-insensitive matching and rejecting extra bytes.
>> It only builds a C string when reporting an invalid form.
>>
>> 0003 replaces the constant-folded VALUES test with stored short and
>> compressed text values.
>>
>> Core regression and the Unicode normalization checks passed with these
>> patches.
>>
>
> OK, that all looks good. The attached combines all this.
Thanks for the updates Andrew. They looks pretty good to me. A few
nitpicks:
> +/*
> + * Return the length of an initial portion of s[0..len) that is valid ASCII,
> + * that is, contains no zero bytes and no bytes with the high bit set, or
> + * len if all of it is.
> + *
> + * is_valid_ascii() requires a length that is a multiple of sizeof(Vector8),
> + * so check one chunk at a time with it and then the remainder byte by byte.
> + * A failure inside a chunk is reported at the start of that chunk, so the
> + * result can be up to sizeof(Vector8) - 1 bytes short of the full valid
> + * ASCII prefix.
> + */
I would relegate that second paragraph to a comment within the function
itself. I am not sure it is valuable to callers, only to those reading
the actual implementation. It also may be a bit verbose, but that isjust
an opinion.
> + /*
> + * Check the remaining code points without first counting them, stopping
> + * at a NUL as pg_mbstrlen_with_len() would.
> + */
> + p = (unsigned char *) VARDATA_ANY(input) + start;
> + end = (unsigned char *) VARDATA_ANY(input) + len;
I am not sure that it makes sense to mention pg_mbstrlen_with_len()
here. I would be more explicit as to why we are stopping at a NUL.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2026-10-06 21:57:02 | Re: Let an ordering index scan hand its ORDER BY value to the target list |
| Previous Message | Tristan Partin | 2026-10-06 21:41:31 | Re: Fix out-of-bounds array indexing in JsonValueList |