Sanding down some edge cases for PL/pgSQL reserved words
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
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
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
"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
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
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 getregression=# 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 RAISEwhich 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
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
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
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 WHILEThis 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
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
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 getregression=# 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 RAISEwhich 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
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
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