Re: Missing semicolumn in anonymous plpgsql block does not raise syntax error

Hi

út 4. 6. 2024 v 8:39 odesílatel Mor Lehr <mor(dot)lehr(at)deel(dot)com> napsal:

> How about inventing an opt-in strict mode
>
>
> That can be useful at the session level, because we use anonymous blocks
> quite often.
> I assume if such a setting existed - we would have used it in the original
> scenario.
>
> Thanks again,
> -Mor
>
> On Mon, Jun 3, 2024 at 9:12 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>
>>
>>
>> po 3. 6. 2024 v 18:46 odesílatel Erik Wienhold <ewie(at)ewie(dot)name> napsal:
>>
>>> On 2024-06-03 00:18 +0200, Tom Lane wrote:
>>> > "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
>>> > > I think you just wrote the equivalent of:
>>> > > l_cnt := (select 1 as delete from foo3 where id=1);
>>> > > Which is a valid query.
>>> >
>>> > Still another example of the folly of letting AS be optional.
>>> > I don't suppose we can ever undo that though.
>>>
>>> How about inventing an opt-in strict mode (like in Perl or JavaScript)
>>> that prevents certain footguns? For example, disallowing bare column
>>> labels.
>>>
>>> That could be enabled for the current session or transaction:
>>>
>>> SET strict_parsing = { on | off };
>>>
>>> Or just for individual routines:
>>>
>>> CREATE PROCEDURE myproc()
>>> SET strict_parsing = { on | off }
>>> LANGUAGE plpgsql ...
>>>
>>>
>> Probably it is not bad idea - it can be generally useful
>>
>> But I think it is better to introduce a new entry for plpgsql expressions
>> in gram.y.
>>
>> Unfortunately it is not a compatible change. Years ago was popular to use
>> a pattern
>>
>> a := tab.a FROM tab
>>
>> instead correct
>>
>> a := (SELECT tab.a FROM tab)
>>
>> or
>>
>> SELECT tab.a FROM tab INTO a;
>>
>> Regards
>>
>> Pavel
>>
>
I wrote an experimental patch that enforces strict syntax for PL/pgSQL
expressions. This patch is a proof concept and shows impacts of the change
(check-world tests passed)

Unfortunately it introduces break compatibility - with this patch the odd
syntax (that is undocumented) is not supported. Something like

DECLARE _x int := x FROM foo WHERE id = _arg;

should not be written in strict mode;

This patch very well fix reported issue:

(2024-06-10 12:06:43) postgres=# CREATE TABLE foo3(id serial PRIMARY key,
txt text);
CREATE TABLE
(2024-06-10 12:07:13) postgres=# INSERT INTO foo3 (txt) VALUES
('aaa'),('bbb');
INSERT 0 2
(2024-06-10 12:07:33) postgres=# DO $$
DECLARE
l_cnt int;
BEGIN
l_cnt := 1
DELETE FROM foo3 WHERE id=1;
END; $$;
ERROR: syntax error at or near "DELETE"
LINE 11: DELETE FROM foo3 WHERE id=1;
^

Personally, I think it can be a strong step forward (we can use deeper
integration of plpgsql with SQL parser which was not possible before).

Because it introduces a compatibility break I don't propose to use it by
default. I see two possible ways, how this check can be used:

1. we can use plpgsql.extra_errors (
https://www.postgresql.org/docs/current/plpgsql-development-tips.html#PLPGSQL-EXTRA-CHECKS
) and introduce a check named (maybe) strict_expr_syntax. Because in this
case the warning cannot be raised, then this check cannot be used in
plpgsql.extra_warnings. I think it can work nicely.

2. if @1 will not be possible, the minimalist implementation can be based
on a new public entry to SQL parser. In this case, I can do the proposed
check at least from plpgsql_check.

Do you see any other possibilities?

This patch is just a proof concept. I didn't implement any mechanism to
switch from default mode to strict mode (I don't propose strict mode as
default) now.

I think it can increase the robustness of plpgsql, on the other hand it
introduces compatibility breaks and I understand related problems.

The change of definition of PLpgSQL_Expr has an impact on OPEN and CASE
PLpgSQL statements that use multicolumn results.

Regards

Pavel

>
>>
>>
>>> --
>>> Erik
>>>
>>>
>>>