Fix missing semicolon in pl_gram.y for option_value rule

Started by shaobo zhangabout 2 months ago10 messageshackers
Jump to latest
#1shaobo zhang
zshaoo224@gmail.com

The option_value grammar rule in src/pl/plpgsql/src/pl_gram.y was
missing a terminating semicolon, which is inconsistent with the style
used by other rules in the file. While Bison does not require
semicolons to terminate grammar rules, adding one here improves
consistency and readability.

Attachments:

v1-0001-Fix-missing-semicolon-in-pl_gram.y-for-option_val.patchapplication/octet-stream; name=v1-0001-Fix-missing-semicolon-in-pl_gram.y-for-option_val.patchDownload+1-1
#2webbohan
webbohan@qq.com
In reply to: shaobo zhang (#1)
回复:Fix missing semicolon in pl_gram.y for option_value rule

Hello,
  The "decl_is_for" syntax block has the same issue.

It can be modified together.

------------------ 原始邮件 ------------------
发件人: "shaobo zhang" <zshaoo224@gmail.com&gt;;
发送时间:&nbsp;2026年6月5日(星期五) 下午3:01
收件人:&nbsp;"pgsql-hackers"<pgsql-hackers@lists.postgresql.org&gt;;

主题:&nbsp;Fix missing semicolon in pl_gram.y for option_value rule

The option_value grammar rule in src/pl/plpgsql/src/pl_gram.y was
missing a terminating semicolon, which is inconsistent with the style
used by other rules in the file. While Bison does not require
semicolons to terminate grammar rules, adding one here improves
consistency&nbsp; and readability.

#3Chao Li
li.evan.chao@gmail.com
In reply to: shaobo zhang (#1)
Re: Fix missing semicolon in pl_gram.y for option_value rule

On Jun 5, 2026, at 15:01, shaobo zhang <zshaoo224@gmail.com> wrote:

The option_value grammar rule in src/pl/plpgsql/src/pl_gram.y was
missing a terminating semicolon, which is inconsistent with the style
used by other rules in the file. While Bison does not require
semicolons to terminate grammar rules, adding one here improves
consistency and readability.
<v1-0001-Fix-missing-semicolon-in-pl_gram.y-for-option_val.patch>

I would not call this a “fix” as it doesn’t fix anything not working today. It is actually a grammar-style cleanup. Since the code being changed is 12 years old, I think this patch is a candidate for v20.

Would you please add it to the CommitFest so that it can be noticed after v20 opens?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#4shaobo zhang
zshaoo224@gmail.com
In reply to: Chao Li (#3)
Re: Fix missing semicolon in pl_gram.y for option_value rule

Hi,

Thank you for the review!

You're right — this is a grammar-style cleanup rather than a fix, since
the existing code works fine without the semicolon. I've reworded the
commit message accordingly to make that clear, and attached it as v2.

I'll add it to the CommitFest as you suggested so it can be picked up
after v20 opens.

Best regards,
shaobo zhang

Chao Li <li.evan.chao@gmail.com> 于2026年6月9日周二 13:52写道:

Show quoted text

On Jun 5, 2026, at 15:01, shaobo zhang <zshaoo224@gmail.com> wrote:

The option_value grammar rule in src/pl/plpgsql/src/pl_gram.y was
missing a terminating semicolon, which is inconsistent with the style
used by other rules in the file. While Bison does not require
semicolons to terminate grammar rules, adding one here improves
consistency and readability.
<v1-0001-Fix-missing-semicolon-in-pl_gram.y-for-option_val.patch>

I would not call this a “fix” as it doesn’t fix anything not working today. It is actually a grammar-style cleanup. Since the code being changed is 12 years old, I think this patch is a candidate for v20.

Would you please add it to the CommitFest so that it can be noticed after v20 opens?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#5shaobo zhang
zshaoo224@gmail.com
In reply to: shaobo zhang (#4)
Re: Fix missing semicolon in pl_gram.y for option_value rule

Oops, I forgot to attach the patch in my previous reply. Attaching v2 now.

Best regards,
shaobo zhang

shaobo zhang <zshaoo224@gmail.com> 于2026年6月9日周二 14:41写道:

Show quoted text

Hi,

Thank you for the review!

You're right — this is a grammar-style cleanup rather than a fix, since
the existing code works fine without the semicolon. I've reworded the
commit message accordingly to make that clear, and attached it as v2.

I'll add it to the CommitFest as you suggested so it can be picked up
after v20 opens.

Best regards,
shaobo zhang

Chao Li <li.evan.chao@gmail.com> 于2026年6月9日周二 13:52写道:

On Jun 5, 2026, at 15:01, shaobo zhang <zshaoo224@gmail.com> wrote:

The option_value grammar rule in src/pl/plpgsql/src/pl_gram.y was
missing a terminating semicolon, which is inconsistent with the style
used by other rules in the file. While Bison does not require
semicolons to terminate grammar rules, adding one here improves
consistency and readability.
<v1-0001-Fix-missing-semicolon-in-pl_gram.y-for-option_val.patch>

I would not call this a “fix” as it doesn’t fix anything not working today. It is actually a grammar-style cleanup. Since the code being changed is 12 years old, I think this patch is a candidate for v20.

Would you please add it to the CommitFest so that it can be noticed after v20 opens?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachments:

v2-0001-plpgsql-Add-missing-terminating-semicolon-to-opti.patchapplication/octet-stream; name=v2-0001-plpgsql-Add-missing-terminating-semicolon-to-opti.patchDownload+1-1
#6Michael Paquier
michael@paquier.xyz
In reply to: Chao Li (#3)
Re: Fix missing semicolon in pl_gram.y for option_value rule

On Tue, Jun 09, 2026 at 12:51:47PM +0800, Chao Li wrote:

I would not call this a “fix” as it doesn’t fix anything not working
today. It is actually a grammar-style cleanup. Since the code being
changed is 12 years old, I think this patch is a candidate for v20.

Why do we even need to care about that?
--
Michael

#7shaobo zhang
zshaoo224@gmail.com
In reply to: Michael Paquier (#6)
Re: Fix missing semicolon in pl_gram.y for option_value rule

On Tue, Jun 09, 2026 at 12:51:47PM +0800, Chao Li wrote:

I would not call this a “fix” as it doesn’t fix anything not working
today. It is actually a grammar-style cleanup. Since the code being
changed is 12 years old, I think this patch is a candidate for v20.

Why do we even need to care about that?
--
Michael

Agreed, it's purely cosmetic. If there's no interest in this cleanup,
I'm fine to withdraw it. Thanks both for looking.

#8Peter Eisentraut
peter_e@gmx.net
In reply to: shaobo zhang (#1)
Re: Fix missing semicolon in pl_gram.y for option_value rule

On 09.06.26 19:03, Tom Lane wrote:

shaobo zhang <zshaoo224@gmail.com> writes:

Why do we even need to care about that?
--
Michael

Agreed, it's purely cosmetic. If there's no interest in this cleanup,
I'm fine to withdraw it. Thanks both for looking.

My inner neatnik likes consistency, so in the abstract I'd rather have
the semicolon present. But I agree with Michael that it's not worth
the overhead to do this sort of fix retail. (BTW I've got little
confidence that there aren't other places needing the same fix.)

What I'd really like is an equivalent of pgindent for bison files,
so that we could enforce this sort of style rule automatically.
But I suppose that's more work than it's worth, too. Unless someone
knows of an existing tool?

The release notes of bison 3.0 say

Missing semicolons at the end of actions are no longer added (as
announced
in the release 2.5).

But apparently that is not in fact happening?

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#8)
Re: Fix missing semicolon in pl_gram.y for option_value rule

Peter Eisentraut <peter@eisentraut.org> writes:

The release notes of bison 3.0 say

Missing semicolons at the end of actions are no longer added (as announced
in the release 2.5).

But apparently that is not in fact happening?

I might be wrong, but I think that is saying that bison will no
longer add a semicolon at the end of the C code for an action.
If you do this:

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 5009e59a78f..238c8ca80f1 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -415,8 +415,9 @@ option_value : T_WORD
                }
             | unreserved_keyword
                {
-                   $$ = pstrdup($1);
+                   $$ = pstrdup($1)
                }
+	;

opt_semi :
| ';'

it doesn't compile. I don't have a pre-3.0 bison to compare to
though.

regards, tom lane

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#9)
Re: Fix missing semicolon in pl_gram.y for option_value rule

On 10.06.26 19:03, Tom Lane wrote:

Peter Eisentraut <peter@eisentraut.org> writes:

The release notes of bison 3.0 say

Missing semicolons at the end of actions are no longer added (as announced
in the release 2.5).

But apparently that is not in fact happening?

I might be wrong, but I think that is saying that bison will no
longer add a semicolon at the end of the C code for an action.
If you do this:

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 5009e59a78f..238c8ca80f1 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -415,8 +415,9 @@ option_value : T_WORD
}
| unreserved_keyword
{
-                   $$ = pstrdup($1);
+                   $$ = pstrdup($1)
}
+	;

opt_semi :
| ';'

it doesn't compile. I don't have a pre-3.0 bison to compare to
though.

Yes, this appears to be correct. With bison 2.3, the above compiles.
The generated code looks like

case 11:
#line 417 "../src/pl/plpgsql/src/pl_gram.y"
{
(yyval.str) = pstrdup((yyvsp[(1) - (1)].keyword))
;}
break;