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

Our documentation describes four allowed input syntaxes for circles,
but the regression tests tried only three ... with predictable
consequences.  Remarkably, this has been wrong since the circle
datatype was added in 1997, but nobody noticed till now.

David Zhang, with some help from me

Discussion: https://postgr.es/m/332c47fa-d951-7574-b5cc-a8f7f7201202@highgo.ca

diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c
index 0348855b11cf6b2d4e4d52fa9fb68ea01af5c25e..da01fe6ba04aa82427b2db6a134183dee7ffac6a 100644 (file)
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -4526,8 +4526,8 @@ poly_path(PG_FUNCTION_ARGS)
 /*             circle_in               -               convert a string to internal form.
  *
  *             External format: (center and radius of circle)
- *                             "((f8,f8)<f8>)"
- *                             also supports quick entry style "(f8,f8,f8)"
+ *                             "<(f8,f8),f8>"
+ *                             also supports quick entry style "f8,f8,f8"
  */
 Datum
 circle_in(PG_FUNCTION_ARGS)
@@ -4541,16 +4541,19 @@ circle_in(PG_FUNCTION_ARGS)
        s = str;
        while (isspace((unsigned char) *s))
                s++;
-       if ((*s == LDELIM_C) || (*s == LDELIM))
+       if (*s == LDELIM_C)
+               depth++, s++;
+       else if (*s == LDELIM)
        {
-               depth++;
+               /* If there are two left parens, consume the first one */
                cp = (s + 1);
                while (isspace((unsigned char) *cp))
                        cp++;
                if (*cp == LDELIM)
-                       s = cp;
+                       depth++, s = cp;
        }
 
+       /* pair_decode will consume parens around the pair, if any */
        pair_decode(s, &circle->center.x, &circle->center.y, &s, "circle", str);
 
        if (*s == DELIM)

diff --git a/src/test/regress/expected/circle.out b/src/test/regress/expected/circle.out
index 9ba4a0495d28cd900196e857d64307e1bb47ffdf..047bc3526502ac15f1b09149cd3cc5791e14cc85 100644 (file)
--- a/src/test/regress/expected/circle.out
+++ b/src/test/regress/expected/circle.out
@@ -3,11 +3,11 @@
 --
 CREATE TABLE CIRCLE_TBL (f1 circle);
 INSERT INTO CIRCLE_TBL VALUES ('<(5,1),3>');
-INSERT INTO CIRCLE_TBL VALUES ('<(1,2),100>');
-INSERT INTO CIRCLE_TBL VALUES ('1,3,5');
-INSERT INTO CIRCLE_TBL VALUES ('((1,2),3)');
-INSERT INTO CIRCLE_TBL VALUES ('<(100,200),10>');
-INSERT INTO CIRCLE_TBL VALUES ('<(100,1),115>');
+INSERT INTO CIRCLE_TBL VALUES ('((1,2),100)');
+INSERT INTO CIRCLE_TBL VALUES (' 1 , 3 , 5 ');
+INSERT INTO CIRCLE_TBL VALUES (' ( ( 1 , 2 ) , 3 ) ');
+INSERT INTO CIRCLE_TBL VALUES (' ( 100 , 200 ) , 10 ');
+INSERT INTO CIRCLE_TBL VALUES (' < ( 100 , 1 ) , 115 > ');
 -- bad values
 INSERT INTO CIRCLE_TBL VALUES ('<(-100,0),-100>');
 ERROR:  invalid input syntax for type circle: "<(-100,0),-100>"

diff --git a/src/test/regress/sql/circle.sql b/src/test/regress/sql/circle.sql
index c0284b2b5989f6c0dd0e3ce71270f61d52477e64..8dd4445b0e6b3ffd15e08c6edd2c694dfb7d9318 100644 (file)
--- a/src/test/regress/sql/circle.sql
+++ b/src/test/regress/sql/circle.sql
@@ -6,15 +6,15 @@ CREATE TABLE CIRCLE_TBL (f1 circle);
 
 INSERT INTO CIRCLE_TBL VALUES ('<(5,1),3>');
 
-INSERT INTO CIRCLE_TBL VALUES ('<(1,2),100>');
+INSERT INTO CIRCLE_TBL VALUES ('((1,2),100)');
 
-INSERT INTO CIRCLE_TBL VALUES ('1,3,5');
+INSERT INTO CIRCLE_TBL VALUES (' 1 , 3 , 5 ');
 
-INSERT INTO CIRCLE_TBL VALUES ('((1,2),3)');
+INSERT INTO CIRCLE_TBL VALUES (' ( ( 1 , 2 ) , 3 ) ');
 
-INSERT INTO CIRCLE_TBL VALUES ('<(100,200),10>');
+INSERT INTO CIRCLE_TBL VALUES (' ( 100 , 200 ) , 10 ');
 
-INSERT INTO CIRCLE_TBL VALUES ('<(100,1),115>');
+INSERT INTO CIRCLE_TBL VALUES (' < ( 100 , 1 ) , 115 > ');
 
 -- bad values