Thread

Commits

  1. Improve some error wording with multirange type parsing

  1. Bracket, brace, parenthesis

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-05-14T06:31:53Z

    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].  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
    
  2. Re: Bracket, brace, parenthesis

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-14T14:04:57Z

    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
    
    
    
    
  3. Re: Bracket, brace, parenthesis

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-05-27T06:20:10Z

    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
    
  4. Re: Bracket, brace, parenthesis

    Michael Paquier <michael@paquier.xyz> — 2021-05-27T12:08:46Z

    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
    
  5. Re: Bracket, brace, parenthesis

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-05-28T06:25:40Z

    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
    
  6. Re: Bracket, brace, parenthesis

    Michael Paquier <michael@paquier.xyz> — 2021-05-31T02:36:23Z

    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
    
  7. Re: Bracket, brace, parenthesis

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-05-31T05:09:16Z

    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