Sanding down some edge cases for PL/pgSQL reserved words

Started by Tom Lane12 months ago11 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

This is a rather delayed response to the discussion of bug
#18693 [1]/messages/by-id/18693-65968418890877b4@postgresql.org, in which I wrote:

(It's kind of annoying that "strict" has to be double-quoted
in the RAISE NOTICE, especially since you get a rather misleading
error if it isn't. But that seems like a different discussion.)

As an example of that, if you don't double-quote "strict"
in this usage you get

regression=# do $$ declare r record; begin
SELECT a, b AS STRICT INTO r FROM (SELECT 'A' AS a, 'B' AS b) AS q;
RAISE NOTICE 'STRICT r.strict = %', r.strict;
end $$;
ERROR: record "r" has no field "strict"
LINE 1: r.strict
^
QUERY: r.strict
CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE

which is pretty bogus because the record *does* have a field
named "strict". The actual problem is that STRICT is a fully
reserved PL/pgSQL keyword, which means you need to double-quote
it if you want to use it this way.

The attached patches provide two independent responses to that:

1. AFAICS, there is no real reason for STRICT to be a reserved
rather than unreserved PL/pgSQL keyword, and for that matter not
EXECUTE either. Making them unreserved does allow some ambiguity,
but I don't think there's any surprises in how that ambiguity
would be resolved; and certainly we've preferred ambiguity over
introducing new reserved keywords in PL/pgSQL before. I think
these two just escaped that treatment by dint of being ancient.

2. That "has no field" error message is flat-out wrong. The now-known
way to trigger it has a different cause, and what's more, we simply do
not know at this point whether the malleable record type has such a
field. So in 0002 below I just changed it to assume that the problem
is a reserved field name. We might find another way to reach that
failure in future, but I doubt that "has no field" would be the right
thing to say in any case.

This is v19 material at this point, so I'll stick it on the CF queue.

regards, tom lane

[1]: /messages/by-id/18693-65968418890877b4@postgresql.org

Attachments:

v1-0001-De-reserve-keywords-EXECUTE-and-STRICT-in-PL-pgSQ.patchtext/x-diff; charset=us-ascii; name*0=v1-0001-De-reserve-keywords-EXECUTE-and-STRICT-in-PL-pgSQ.p; name*1=atchDownload+12-8
v1-0002-Improve-error-report-for-PL-pgSQL-reserved-word-u.patchtext/x-diff; charset=us-ascii; name*0=v1-0002-Improve-error-report-for-PL-pgSQL-reserved-word-u.p; name*1=atchDownload+50-8
#2Joel Jacobson
joel@compiler.org
In reply to: Tom Lane (#1)
Re: Sanding down some edge cases for PL/pgSQL reserved words

On Sat, Apr 26, 2025, at 06:44, Tom Lane wrote:

This is a rather delayed response to the discussion of bug
#18693 [1], in which I wrote:

...

which is pretty bogus because the record *does* have a field
named "strict". The actual problem is that STRICT is a fully
reserved PL/pgSQL keyword, which means you need to double-quote
it if you want to use it this way.

I'd like to briefly raise an old nostalgic PL/pgSQL dream of mine that might be
affected by this change.

For years, I've felt we could benefit from introducing convenience syntax to
explicitly require that exactly one row is affected by a query, something which
currently requires using a somewhat cumbersome workaround:

- Using `... INTO STRICT ...` for `SELECT`,
- Using `RETURNING ... INTO STRICT ...` for `DELETE/UPDATE/INSERT`, or
- Checking `ROW_COUNT` via `GET DIAGNOSTICS` and raising an error if not exactly one row.

I think it would be more convenient and intuitive if we could simply write:

```
STRICT [SELECT | UPDATE | INSERT | DELETE] ...;
```

That is, allowing `STRICT` followed directly by any regular `SELECT`, `UPDATE`,
`INSERT`, or `DELETE` command, explicitly enforcing exactly one affected row.

Changing `STRICT` to become an unreserved keyword in PL/pgSQL would effectively
close the window of opportunity for this syntax, as it would introduce ambiguity
in command parsing.

I was actually not aware of STRICT already being a reserved PL/pgSQL keyword.
Had I known that, I would have proposed this convenience syntax already since
a long time ago.

I wonder how often developers truly need to use "strict" as a field name versus
the potential usage of a clean and explicit syntax for enforcing single-row
results without additional verbosity.

/Joel

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joel Jacobson (#2)
Re: Sanding down some edge cases for PL/pgSQL reserved words

"Joel Jacobson" <joel@compiler.org> writes:

For years, I've felt we could benefit from introducing convenience syntax to
explicitly require that exactly one row is affected by a query, something which
currently requires using a somewhat cumbersome workaround:

- Using `... INTO STRICT ...` for `SELECT`,
- Using `RETURNING ... INTO STRICT ...` for `DELETE/UPDATE/INSERT`, or
- Checking `ROW_COUNT` via `GET DIAGNOSTICS` and raising an error if not exactly one row.

I think it would be more convenient and intuitive if we could simply write:

```
STRICT [SELECT | UPDATE | INSERT | DELETE] ...;
```

Meh. I don't really have an opinion on whether this is worth bespoke
syntax, but if it is:

(1) I don't see why we'd restrict it to plpgsql as opposed to
implementing it in core SQL.

(2) Putting the keyword at the front seems fairly un-SQL-like.
For SELECT, "SELECT STRICT ..." would seem more natural, as it calls
back to SELECT DISTINCT; or you could imagine integrating it into the
LIMIT clause. Not as sure what to do for the DML commands, but
somewhere near where we put RETURNING seems saner.

Also, even if we did do it in plpgsql exactly as you suggest, making
it unreserved doesn't move the needle on whether that's possible.
Most of plpgsql's statement-starting keywords are unreserved.

But please, don't hijack this thread for that discussion ...

regards, tom lane

#4Joel Jacobson
joel@compiler.org
In reply to: Tom Lane (#3)
Re: Sanding down some edge cases for PL/pgSQL reserved words

On Sat, Apr 26, 2025, at 05:10, Tom Lane wrote:

"Joel Jacobson" <joel@compiler.org> writes:

For years, I've felt we could benefit from introducing convenience syntax to
explicitly require that exactly one row is affected by a query, something which
currently requires using a somewhat cumbersome workaround:

- Using `... INTO STRICT ...` for `SELECT`,
- Using `RETURNING ... INTO STRICT ...` for `DELETE/UPDATE/INSERT`, or
- Checking `ROW_COUNT` via `GET DIAGNOSTICS` and raising an error if not exactly one row.

I think it would be more convenient and intuitive if we could simply write:

```
STRICT [SELECT | UPDATE | INSERT | DELETE] ...;
```

Meh. I don't really have an opinion on whether this is worth bespoke
syntax, but if it is:

(1) I don't see why we'd restrict it to plpgsql as opposed to
implementing it in core SQL.

Good point, I agree, that would be much better.

(2) Putting the keyword at the front seems fairly un-SQL-like.
For SELECT, "SELECT STRICT ..." would seem more natural, as it calls
back to SELECT DISTINCT; or you could imagine integrating it into the
LIMIT clause. Not as sure what to do for the DML commands, but
somewhere near where we put RETURNING seems saner.

Also, even if we did do it in plpgsql exactly as you suggest, making
it unreserved doesn't move the needle on whether that's possible.
Most of plpgsql's statement-starting keywords are unreserved.

But please, don't hijack this thread for that discussion ...

Understood, and thanks for clarifying this change doesn't affect the strictness idea.

/Joel

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#1)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Hi

I started reviewing this patch.

so 7. 6. 2025 v 18:41 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

This is a rather delayed response to the discussion of bug
#18693 [1], in which I wrote:

(It's kind of annoying that "strict" has to be double-quoted
in the RAISE NOTICE, especially since you get a rather misleading
error if it isn't. But that seems like a different discussion.)

As an example of that, if you don't double-quote "strict"
in this usage you get

regression=# do $$ declare r record; begin
SELECT a, b AS STRICT INTO r FROM (SELECT 'A' AS a, 'B' AS b) AS q;
RAISE NOTICE 'STRICT r.strict = %', r.strict;
end $$;
ERROR: record "r" has no field "strict"
LINE 1: r.strict
^
QUERY: r.strict
CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE

which is pretty bogus because the record *does* have a field
named "strict". The actual problem is that STRICT is a fully
reserved PL/pgSQL keyword, which means you need to double-quote
it if you want to use it this way.

The attached patches provide two independent responses to that:

1. AFAICS, there is no real reason for STRICT to be a reserved
rather than unreserved PL/pgSQL keyword, and for that matter not
EXECUTE either. Making them unreserved does allow some ambiguity,
but I don't think there's any surprises in how that ambiguity
would be resolved; and certainly we've preferred ambiguity over
introducing new reserved keywords in PL/pgSQL before. I think
these two just escaped that treatment by dint of being ancient.

There is no issue.

2. That "has no field" error message is flat-out wrong. The now-known
way to trigger it has a different cause, and what's more, we simply do
not know at this point whether the malleable record type has such a
field. So in 0002 below I just changed it to assume that the problem
is a reserved field name. We might find another way to reach that
failure in future, but I doubt that "has no field" would be the right
thing to say in any case.

The proposed patch is a zero invasive solution. But the question is why we
cannot allow plpgsql reserved keywords in recfilds?

There should not be any collisions. Isn't there a better solution to
modify plpgsql_yylex instead and allow all keywords after '.' ? Sure. It
will be more invasive.

Regards

Pavel

Show quoted text

This is v19 material at this point, so I'll stick it on the CF queue.

regards, tom lane

[1]
/messages/by-id/18693-65968418890877b4@postgresql.org

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#5)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Hi

2. That "has no field" error message is flat-out wrong. The now-known
way to trigger it has a different cause, and what's more, we simply do
not know at this point whether the malleable record type has such a
field. So in 0002 below I just changed it to assume that the problem
is a reserved field name. We might find another way to reach that
failure in future, but I doubt that "has no field" would be the right
thing to say in any case.

The proposed patch is a zero invasive solution. But the question is why we
cannot allow plpgsql reserved keywords in recfilds?

There should not be any collisions. Isn't there a better solution to
modify plpgsql_yylex instead and allow all keywords after '.' ? Sure. It
will be more invasive.

Is there some description of what keywords should be reserved? If I
remember correctly, the scanner was changed more times, and maybe more
reserved keywords are not necessary.

Regards

Pavel

Show quoted text

Regards

Pavel

This is v19 material at this point, so I'll stick it on the CF queue.

regards, tom lane

[1]
/messages/by-id/18693-65968418890877b4@postgresql.org

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#6)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Pavel Stehule <pavel.stehule@gmail.com> writes:

Is there some description of what keywords should be reserved? If I
remember correctly, the scanner was changed more times, and maybe more
reserved keywords are not necessary.

Per the comment in pl_scanner.c:

* We try to avoid reserving more keywords than we have to; but there's
* little point in not reserving a word if it's reserved in the core grammar.
* Currently, the following words are reserved here but not in the core:
* BEGIN BY DECLARE EXECUTE FOREACH IF LOOP STRICT WHILE

This patch gets rid of EXECUTE and STRICT, but the others are harder
to de-reserve. I think most of the rest are there because they can
follow a block or loop label, and the same comment observes

* (We still have to reserve initial keywords that might follow a block
* label, unfortunately, since the method used to determine if we are at
* start of statement doesn't recognize such cases.

regards, tom lane

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#7)
Re: Sanding down some edge cases for PL/pgSQL reserved words

ne 8. 6. 2025 v 23:49 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

Is there some description of what keywords should be reserved? If I
remember correctly, the scanner was changed more times, and maybe more
reserved keywords are not necessary.

Per the comment in pl_scanner.c:

* We try to avoid reserving more keywords than we have to; but there's
* little point in not reserving a word if it's reserved in the core
grammar.
* Currently, the following words are reserved here but not in the core:
* BEGIN BY DECLARE EXECUTE FOREACH IF LOOP STRICT WHILE

This patch gets rid of EXECUTE and STRICT, but the others are harder
to de-reserve. I think most of the rest are there because they can
follow a block or loop label, and the same comment observes

* (We still have to reserve initial keywords that might follow a block
* label, unfortunately, since the method used to determine if we are at
* start of statement doesn't recognize such cases.

Looks so block label is a problem, but loop label not - and then BEGIN
DECLARE WHEN is really required reserved world
by gram.y

Maybe these comments are a little bit obsolete. Probably is not a good idea
to make unreserved words keywords used
as read_sql_xxxx delimiter: WHEN, LOOP, WHILE, INTO, USING, IN, FROM, and
maybe some other. This is probably
main reason why PL/pgSQL has these keywords marked as reserved.

Maybe there should be a new assert, that checks so the keywords used as
delimiters are reserved keywords.

I checked the list of reserved words of Ada language or PL/SQL language and
we are significantly different.

I can imagine two situations.

a) current state + Tom's patch that reports so keywords are reserved

b) ignore the keyword after the "dot" symbol, and allow the reserved
keyword as a record field without limits. SQL now allows using a lot of
keywords as labels without
necessity of using AS or double quoting.

Both variants can work well I think - a) is more strict, zero invasive, b)
is more user friendly, but small typo can hide some problems.

What do you think about it?

Regards

Pavel

Show quoted text

regards, tom lane

#9Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#1)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Hi

1. AFAICS, there is no real reason for STRICT to be a reserved
rather than unreserved PL/pgSQL keyword, and for that matter not
EXECUTE either. Making them unreserved does allow some ambiguity,
but I don't think there's any surprises in how that ambiguity
would be resolved; and certainly we've preferred ambiguity over
introducing new reserved keywords in PL/pgSQL before. I think
these two just escaped that treatment by dint of being ancient.

I checked other reserved keywords and I didn't see any reason to be
reserved keywords
for K_TO, K_NOT.

K_FOREACH, and K_WHILE are reserved probably because are used after
opt_loop_label - but it is not necessary

Other keywords are used as some delimiter or as protection against parser's
conflicts.

Regards

Pavel

#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#5)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Hi

ne 8. 6. 2025 v 6:25 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

Hi

I started reviewing this patch.

so 7. 6. 2025 v 18:41 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

This is a rather delayed response to the discussion of bug
#18693 [1], in which I wrote:

(It's kind of annoying that "strict" has to be double-quoted
in the RAISE NOTICE, especially since you get a rather misleading
error if it isn't. But that seems like a different discussion.)

As an example of that, if you don't double-quote "strict"
in this usage you get

regression=# do $$ declare r record; begin
SELECT a, b AS STRICT INTO r FROM (SELECT 'A' AS a, 'B' AS b) AS q;
RAISE NOTICE 'STRICT r.strict = %', r.strict;
end $$;
ERROR: record "r" has no field "strict"
LINE 1: r.strict
^
QUERY: r.strict
CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE

which is pretty bogus because the record *does* have a field
named "strict". The actual problem is that STRICT is a fully
reserved PL/pgSQL keyword, which means you need to double-quote
it if you want to use it this way.

The attached patches provide two independent responses to that:

1. AFAICS, there is no real reason for STRICT to be a reserved
rather than unreserved PL/pgSQL keyword, and for that matter not
EXECUTE either. Making them unreserved does allow some ambiguity,
but I don't think there's any surprises in how that ambiguity
would be resolved; and certainly we've preferred ambiguity over
introducing new reserved keywords in PL/pgSQL before. I think
these two just escaped that treatment by dint of being ancient.

There is no issue.

2. That "has no field" error message is flat-out wrong. The now-known
way to trigger it has a different cause, and what's more, we simply do
not know at this point whether the malleable record type has such a
field. So in 0002 below I just changed it to assume that the problem
is a reserved field name. We might find another way to reach that
failure in future, but I doubt that "has no field" would be the right
thing to say in any case.

The proposed patch is a zero invasive solution. But the question is why we
cannot allow plpgsql reserved keywords in recfilds?

There should not be any collisions. Isn't there a better solution to
modify plpgsql_yylex instead and allow all keywords after '.' ? Sure. It
will be more invasive.

Looks so nobody has any motivation to do some deeper changes to reduce
prohibition of reserved words. It is true, so in the real world it is not
an issue.

I did a review, and I didn't find any issue.

All tests passed without problems. I'll mark this patch as ready for commit.

Maybe the usage of unreserved words as variables or field names can be
tested a little bit more. See patch 0003

Regards

Pavel

Show quoted text

Regards

Pavel

This is v19 material at this point, so I'll stick it on the CF queue.

regards, tom lane

[1]
/messages/by-id/18693-65968418890877b4@postgresql.org

Attachments:

0002-Improve-error-report-for-PL-pgSQL-reserved-word-used.patchtext/x-patch; charset=US-ASCII; name=0002-Improve-error-report-for-PL-pgSQL-reserved-word-used.patchDownload+50-8
0003-test-of-usage-of-unreserved-words-as-variables-or-re.patchtext/x-patch; charset=US-ASCII; name=0003-test-of-usage-of-unreserved-words-as-variables-or-re.patchDownload+27-1
0001-De-reserve-keywords-EXECUTE-and-STRICT-in-PL-pgSQL.patchtext/x-patch; charset=US-ASCII; name=0001-De-reserve-keywords-EXECUTE-and-STRICT-in-PL-pgSQL.patchDownload+12-8
#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#10)
Re: Sanding down some edge cases for PL/pgSQL reserved words

Pavel Stehule <pavel.stehule@gmail.com> writes:

Looks so nobody has any motivation to do some deeper changes to reduce
prohibition of reserved words. It is true, so in the real world it is not
an issue.

Certainly anyone who's annoyed is free to do more work here.

All tests passed without problems. I'll mark this patch as ready for commit.
Maybe the usage of unreserved words as variables or field names can be
tested a little bit more. See patch 0003

Pushed with the test case as you suggested. Thanks for reviewing!

regards, tom lane