Re: Latest patches break one of our unit-test, related to RLS

On Fri, 2025-09-12 at 20:12 -0400, Tom Lane wrote:
> I had not particularly studied the new charclass-parsing logic.
> Looking at it now, this bit further down (lines 871ff) looks
> fishy:
>
>             if (pchar == ']' && charclass_start > 2)
>                 charclass_depth--;
>             else if (pchar == '[')
>                 charclass_depth++;
>
>             /*
>              * If there is a caret right after the opening bracket, it negates
>              * the character class, but a following closing bracket should
>              * still be treated as a normal character.  That holds only for
>              * the first caret, so only the values 1 and 2 mean that closing
>              * brackets should be taken literally.
>              */
>             if (pchar == '^')
>                 charclass_start++;
>             else
>                 charclass_start = 3;    /* definitely past the start */
>
> Should not we be setting charclass_start to 1 after incrementing
> charclass_depth?

What I call "charclass depth" is misleading, I am afraid.
Really, it should be "bracket depth". Only the outermost pair of brackets
starts an actual character class. Example:

[]abc[:digit:]]

A caret or closing bracket right after the inner opening bracket wouldn't
be a special character, and I think it would never be legal.

Unfortunately, this is all pretty complicated.

Perhaps s/charclass_depth/bracket_depth/ would be a good idea.

Yours,
Laurenz Albe