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

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Fri, 2025-09-12 at 20:12 -0400, Tom Lane wrote:
>> 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.

Ah, got it.  But this logic definitely deserves more comments.
What do you think of something like

            if (pchar == ']' && charclass_start > 2)
            {
                /* found the real end of a bracket pair */
                charclass_depth--;
                /* past start of outer brackets, so keep charclass_start > 2 */
            }
            else if (pchar == '[')
            {
                /* start of a nested bracket pair */
                charclass_depth++;
                /* leading ^ or ] in this context is not special */
                charclass_start = 3;
            }
            else if (pchar == '^')
            {
                /* okay to increment charclass_start even if already > 1 */
                charclass_start++;
            }
            else
            {
                /* otherwise (including case of leading ']') */
                charclass_start = 3;    /* definitely past the start */
            }

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

Wouldn't object to that.  Maybe we can think of a better name for
"charclass_start" too --- that sounds like a boolean condition.
The best I'm coming up with right now is "charclass_count", but
that's not that helpful.

			regards, tom lane