Thread: ERROR: invalid input syntax for type circle
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
Attachment
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
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
Attachment
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