Horton

Thread

Commits

  1. Fix circle_in to accept "(x,y),r" as it's advertised to do.

  1. ERROR: invalid input syntax for type circle

    David Zhang <david.zhang@highgo.ca> — 2020-04-06T21:12:38Z

    Hi,

I got an error when I was trying to insert a circle using the syntax 
(the 3rd one) specified in the latest document.

https://www.postgresql.org/docs/current/datatype-geometric.html#DATATYPE-CIRCLE
< ( x , y ) , r >
( ( x , y ) , r )
   ( x , y ) , r
     x , y   , r

Here is how to reproduce it.

CREATE TABLE tbl_circle(id serial PRIMARY KEY, a circle);
INSERT INTO tbl_circle(a) VALUES('( 1 , 1 ) , 5'::circle );

ERROR:  invalid input syntax for type circle: "( 1 , 1 ) , 5"
LINE 1: INSERT INTO tbl_circle(a) VALUES('( 1 , 1 ) , 5'::circle );

I made a little change in the "circle_in" function, and then I can enter 
a circle using the 3rd way.

INSERT INTO tbl_circle(a) VALUES('( 1 , 1 ) , 5'::circle );
INSERT 0 1

The fix does generate the same output as the other three ways.

INSERT INTO tbl_circle(a) VALUES( '< ( 1 , 1 ) , 5 >'::circle );
INSERT INTO tbl_circle(a) VALUES( '( ( 1 , 1 ) , 5 )'::circle );
INSERT INTO tbl_circle(a) VALUES( '1 , 1 , 5'::circle );

select * from tbl_circle;
  id |     a
----+-----------
   1 | <(1,1),5>
   2 | <(1,1),5>
   3 | <(1,1),5>
   4 | <(1,1),5>
(4 rows)

Sor far, no error found during the "make check".

The patch based on tag "REL_12_2" is attached.

-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca

  2. Re: ERROR: invalid input syntax for type circle

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-04-06T22:16:37Z

    David Zhang <david.zhang@highgo.ca> writes:
> I got an error when I was trying to insert a circle using the syntax 
> (the 3rd one) specified in the latest document.

Hm.  Presumably, that has never worked, and we've had no complaints
to date.  I'm halfway inclined to treat it as a documentation bug
and remove the claim that it works.

> The patch based on tag "REL_12_2" is attached.

This patch looks extremely dangerous to me, because it'll allow "s"
to get incremented past the ending nul character ... and then the
code will proceed to keep scanning, which at best is useless and
at worst will end in a core dump.

What actually looks wrong to me in this code is the initial bit

    if ((*s == LDELIM_C) || (*s == LDELIM))
    {
        depth++;
        cp = (s + 1);
        while (isspace((unsigned char) *cp))
            cp++;
        if (*cp == LDELIM)
            s = cp;
    }

If the first test triggers but it doesn't then find a following
paren, then it's incremented depth without moving s, which seems
certain to end badly.  Perhaps the correct fix is like

    if (*s == LDELIM_C)
        depth++, s++;
    else if (*s == LDELIM)
    {
        /* If there are two left parens, consume the first one */
        cp = (s + 1);
        while (isspace((unsigned char) *cp))
            cp++;
        if (*cp == LDELIM)
            depth++, s = cp;
    }

			regards, tom lane

  3. Re: ERROR: invalid input syntax for type circle

    David Zhang <david.zhang@highgo.ca> — 2020-04-07T00:44:05Z

    Hi Tom,

Thanks for the review.

Generated a new patch v2 (attached) following your suggestion and 
performed the same test again. The test results looks good including the 
"make check".

On 2020-04-06 3:16 p.m., Tom Lane wrote:
> David Zhang <david.zhang@highgo.ca> writes:
>> I got an error when I was trying to insert a circle using the syntax
>> (the 3rd one) specified in the latest document.
> Hm.  Presumably, that has never worked, and we've had no complaints
> to date.  I'm halfway inclined to treat it as a documentation bug
> and remove the claim that it works.
>
>> The patch based on tag "REL_12_2" is attached.
> This patch looks extremely dangerous to me, because it'll allow "s"
> to get incremented past the ending nul character ... and then the
> code will proceed to keep scanning, which at best is useless and
> at worst will end in a core dump.
>
> What actually looks wrong to me in this code is the initial bit
>
>      if ((*s == LDELIM_C) || (*s == LDELIM))
>      {
>          depth++;
>          cp = (s + 1);
>          while (isspace((unsigned char) *cp))
>              cp++;
>          if (*cp == LDELIM)
>              s = cp;
>      }
>
> If the first test triggers but it doesn't then find a following
> paren, then it's incremented depth without moving s, which seems
> certain to end badly.  Perhaps the correct fix is like
>
>      if (*s == LDELIM_C)
>          depth++, s++;
>      else if (*s == LDELIM)
>      {
>          /* If there are two left parens, consume the first one */
>          cp = (s + 1);
>          while (isspace((unsigned char) *cp))
>              cp++;
>          if (*cp == LDELIM)
>              depth++, s = cp;
>      }
>
> 			regards, tom lane
-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca

  4. Re: ERROR: invalid input syntax for type circle

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-04-08T00:51:26Z

    David Zhang <david.zhang@highgo.ca> writes:
> Generated a new patch v2 (attached) following your suggestion and 
> performed the same test again. The test results looks good including the 
> "make check".

Pushed, with some work on the regression tests so this doesn't get
busted again.

			regards, tom lane