On 2026-10-06 Tu 5:56 PM, Tristan Partin wrote:
> 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.
Thanks. I'm intending to commit this during the November CF, will fix then.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
