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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Dominique Devienne <ddevienne(at)gmail(dot)com>, pgsql-general(at)postgresql(dot)org
Subject: Re: Latest patches break one of our unit-test, related to RLS
Date: 2025-09-13 01:53:17
Message-ID: 2683918.1757728397@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-general

Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)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

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Justin 2025-09-13 01:54:44 Re: MVCC and all that...
Previous Message Laurenz Albe 2025-09-13 01:17:17 Re: Latest patches break one of our unit-test, related to RLS