Re: CASE control block broken by a single line comment

From: Erik Wienhold <ewie(at)ewie(dot)name>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michal Bartak <maxym(dot)srpl(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: CASE control block broken by a single line comment
Date: 2024-04-07 14:55:27
Message-ID: 5pdlyhplp7c4kzp7aakrzor2fbtuuygjwr25okim5ye7bz33hi@r6itkn735q65
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-04-07 06:33 +0200, Tom Lane wrote:
> Erik Wienhold <ewie(at)ewie(dot)name> writes:
> > I'm surprised that the comment is not skipped by the scanner at this
> > point. Maybe because the parser just reads the raw expression between
> > WHEN and THEN with plpgsql_append_source_text via read_sql_construct.
>
> > How about the attached patch? It's a workaround by simply adding a line
> > feed character between the raw expression and the closing parenthesis.
>
> I don't have time to look into this on this deadline weekend,

Sure, no rush.

> but what's bothering me about this report is the worry that we've made
> the same mistake elsewhere, or will do so in future.

Right. At the moment only make_case is affected by this because it uses
the raw expression for rewriting. I checked other uses of
read_psql_construct (e.g. IF ... THEN, FOR ... LOOP) and they don't show
this bug.

> I suspect it'd be much more robust if we could remove the comment from
> the expr->query string. No idea how hard that is.

I slept on it and I think this can be fixed by tracking the end of the
last token before THEN and use that instead of yylloc in the call to
plpgsql_append_source_text. We already already track the token length
in plpgsql_yyleng but don't make it available outside pl_scanner.c yet.

Attached v2 tries to do that. But it breaks other test cases, probably
because the calculation of endlocation is off. I'm missing something
here.

--
Erik

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Wienhold 2024-04-07 14:56:53 Re: CASE control block broken by a single line comment
Previous Message Melanie Plageman 2024-04-07 14:54:56 Re: BitmapHeapScan streaming read user and prelim refactoring