Re: Support POSITION with nondeterministic collations

On 12.02.25 05:31, Euler Taveira wrote:
> On Mon, Dec 2, 2024, at 6:09 AM, Peter Eisentraut wrote:
>> On 26.08.24 08:09, Peter Eisentraut wrote:
>> > This patch allows using text position search functions with
>> > nondeterministic collations.  These functions are
>> >
>> > - position, strpos
>> > - replace
>> > - split_part
>> > - string_to_array
>> > - string_to_table
>> >
>> > which all use common internal infrastructure.
>>
>> > Some exploratory testing could be useful here.  The present test
>> > coverage was already quite helpful during development, but there is
>> > always the possibility that something was overlooked.
> 
> I took a look at this patch.
> 
>           * Most callers will require "greedy" semantics, meaning that 
> we need
>           * to find the longest such substring, not the shortest.  For 
> callers
>           * don't don't need greedy semantics, we can finish on the first
> 
> s/don't don't/that don't/ ?

fixed

> -   Assert(len1 > 0);
>      Assert(len2 > 0);
> 
> Is there a reason to remove this assert?

len1 is the length of the "haystack".  The previous code did not call 
text_position_setup() with an empty haystack, due to early exits at

/* Empty needle always matches at position 1 */

and

/* Otherwise, can't match if haystack is shorter than needle */

but the second one does not apply for nondeterministic collations, so we 
have to deal with with zero-length haystacks.

>       * (With nondeterministic collations, the search was already
>       * multibyte-aware, so we don't need this.)
> 
> s/was/is/

fixed

> The commit title could be changed to reflect that you are adding support for
> multiple functions. The POSITION gives the impression that it is only 
> for the
> position() function. Something like
> 
>      Support position search functions with nondeterministic collations

fixed

> I did a couple of tests (some are shown below) and I didn't find issues.

Thanks for the extra tests.  I have committed this with the adjustments 
mentioned above.