Bracket, brace, parenthesis

Started by Kyotaro Horiguchialmost 5 years ago7 messageshackers
Jump to latest
#1Kyotaro Horiguchi
horikyota.ntt@gmail.com

I found the following code in multirangetypes.c

if (*ptr == '{')
ptr++;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("malformed multirange literal: \"%s\"",
input_str),
errdetail("Missing left bracket.")));

I'm not sure how much we (or people) are strcit on the distinction
between the $SUBJECT, isn't '{' a brace generally?

postgres=# select '[1,3]'::int4multirange;
ERROR: malformed multirange literal: "[1,3]"
LINE 1: select '[1,3]'::int4multirange;
^
DETAIL: Missing left bracket.

The distinction is significant there. It should at least be "Missing
left curly bracket." or "Missing left brace." (or left curly brace..?)

'{' is mentioned as "curly brackets" in comments of the soruce file.
It is mentioned as "brace" in regexp doc [1]https://www.postgresql.org/docs/devel/functions-matching.html. And.. uh.. I found the
world "curly braces" in the doc for conding conventions..

[1]: https://www.postgresql.org/docs/devel/functions-matching.html

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

brace_not_bracket.patchtext/x-patch; charset=us-asciiDownload+2-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kyotaro Horiguchi (#1)
Re: Bracket, brace, parenthesis

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

I'm not sure how much we (or people) are strcit on the distinction
between the $SUBJECT, isn't '{' a brace generally?

+1. I tend to write "square bracket" or "curly brace" when I want to
be extra clear, but I think the bare terms are widely understood to
have those meanings.

regards, tom lane

#3Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Tom Lane (#2)
Re: Bracket, brace, parenthesis

At Fri, 14 May 2021 10:04:57 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

I'm not sure how much we (or people) are strcit on the distinction
between the $SUBJECT, isn't '{' a brace generally?

+1. I tend to write "square bracket" or "curly brace" when I want to
be extra clear, but I think the bare terms are widely understood to
have those meanings.

Thanks! I think the message is new in 14 so we can fix it right
away. The attached is the version with a commit message added.

If not, I'll register this to the next CF.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Change-confusing-bracket-use-to-clearer-wording.patchtext/x-patch; charset=us-asciiDownload+2-3
#4Michael Paquier
michael@paquier.xyz
In reply to: Kyotaro Horiguchi (#3)
Re: Bracket, brace, parenthesis

On Thu, May 27, 2021 at 03:20:10PM +0900, Kyotaro Horiguchi wrote:

At Fri, 14 May 2021 10:04:57 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in

+1. I tend to write "square bracket" or "curly brace" when I want to
be extra clear, but I think the bare terms are widely understood to
have those meanings.

Thanks! I think the message is new in 14 so we can fix it right
away. The attached is the version with a commit message added.

No objections from me to fix that on HEAD now for clarity, let's wait
a bit and see if others have more comments. You have missed an update
of multirangetypes.out, by the way.
--
Michael

#5Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Michael Paquier (#4)
Re: Bracket, brace, parenthesis

At Thu, 27 May 2021 21:08:46 +0900, Michael Paquier <michael@paquier.xyz> wrote in

On Thu, May 27, 2021 at 03:20:10PM +0900, Kyotaro Horiguchi wrote:

At Fri, 14 May 2021 10:04:57 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in

+1. I tend to write "square bracket" or "curly brace" when I want to
be extra clear, but I think the bare terms are widely understood to
have those meanings.

Thanks! I think the message is new in 14 so we can fix it right
away. The attached is the version with a commit message added.

No objections from me to fix that on HEAD now for clarity, let's wait
a bit and see if others have more comments. You have missed an update
of multirangetypes.out, by the way.

Mmm. Thanks. So the test doesn't a check for the case of trailing
garbage. Looking the discussion about trailing garbage of interger
values, we might need one for the case.

The atached second file adds a test for trailing garbage for
multirangetype.sql and rangetype.sql.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

v2-0001-Change-confusing-bracket-use-to-clearer-wording.patchtext/x-patch; charset=us-asciiDownload+3-4
v2-0002-Add-test-cases-for-trailing-garbage-of-multi-rang.patchtext/x-patch; charset=us-asciiDownload+12-1
#6Michael Paquier
michael@paquier.xyz
In reply to: Kyotaro Horiguchi (#5)
Re: Bracket, brace, parenthesis

On Fri, May 28, 2021 at 03:25:40PM +0900, Kyotaro Horiguchi wrote:

Mmm. Thanks. So the test doesn't a check for the case of trailing
garbage. Looking the discussion about trailing garbage of integer
values, we might need one for the case.

The atached second file adds a test for trailing garbage for
multirangetype.sql and rangetype.sql.

True for the lack of coverage with some junk after the right brace for
multi-ranges, but rangetypes.sql has already some coverage. Applied
with this small update.
--
Michael

#7Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Michael Paquier (#6)
Re: Bracket, brace, parenthesis

At Mon, 31 May 2021 11:36:23 +0900, Michael Paquier <michael@paquier.xyz> wrote in

On Fri, May 28, 2021 at 03:25:40PM +0900, Kyotaro Horiguchi wrote:

Mmm. Thanks. So the test doesn't a check for the case of trailing
garbage. Looking the discussion about trailing garbage of integer
values, we might need one for the case.

The atached second file adds a test for trailing garbage for
multirangetype.sql and rangetype.sql.

True for the lack of coverage with some junk after the right brace for
multi-ranges, but rangetypes.sql has already some coverage. Applied
with this small update.

Hmm. Right. Thanks for the check and commiting!

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center