Shapes on the regression test for polygon

Started by Emre Hasegeliover 11 years ago10 messages
#1Emre Hasegeli
emre@hasegeli.com
1 attachment(s)

The first two shapes on src/test/regress/sql/polygon.sql do not make
sense to me. They look more like polygons with some more tabs,
but still did not match the coordinates. I changed them to make
consistent with the shapes. I believe this was the intention of
the original author. Patch attached.

Attachments:

regress-polygon-fix.sqltext/plain; charset=utf-8Download
diff --git a/src/test/regress/expected/polygon.out b/src/test/regress/expected/polygon.out
index b252902..66ff51d 100644
--- a/src/test/regress/expected/polygon.out
+++ b/src/test/regress/expected/polygon.out
@@ -1,28 +1,28 @@
 --
 -- POLYGON
 --
 -- polygon logic
 --
 -- 3	      o
---	      |
+--		      |
 -- 2	    + |
---	   /  |
--- 1	  # o +
---       /    |
+--		   /  |
+-- 1	  #   +
+--       /  o |
 -- 0	#-----o-+
 --
---	0 1 2 3 4
+--		0 1 2 3 4
 --
 CREATE TABLE POLYGON_TBL(f1 polygon);
-INSERT INTO POLYGON_TBL(f1) VALUES ('(2.0,0.0),(2.0,4.0),(0.0,0.0)');
-INSERT INTO POLYGON_TBL(f1) VALUES ('(3.0,1.0),(3.0,3.0),(1.0,0.0)');
+INSERT INTO POLYGON_TBL(f1) VALUES ('(2.0,2.0),(0.0,0.0),(4.0,0.0)');
+INSERT INTO POLYGON_TBL(f1) VALUES ('(3.0,3.0),(1.0,1.0),(3.0,0.0)');
 -- degenerate polygons
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0,0.0)');
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0,1.0),(0.0,1.0)');
 -- bad polygon input strings
 INSERT INTO POLYGON_TBL(f1) VALUES ('0.0');
 ERROR:  invalid input syntax for type polygon: "0.0"
 LINE 1: INSERT INTO POLYGON_TBL(f1) VALUES ('0.0');
                                             ^
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0 0.0');
 ERROR:  invalid input syntax for type polygon: "(0.0 0.0"
@@ -36,157 +36,170 @@ INSERT INTO POLYGON_TBL(f1) VALUES ('(0,1,2,3');
 ERROR:  invalid input syntax for type polygon: "(0,1,2,3"
 LINE 1: INSERT INTO POLYGON_TBL(f1) VALUES ('(0,1,2,3');
                                             ^
 INSERT INTO POLYGON_TBL(f1) VALUES ('asdf');
 ERROR:  invalid input syntax for type polygon: "asdf"
 LINE 1: INSERT INTO POLYGON_TBL(f1) VALUES ('asdf');
                                             ^
 SELECT '' AS four, * FROM POLYGON_TBL;
  four |         f1          
 ------+---------------------
-      | ((2,0),(2,4),(0,0))
-      | ((3,1),(3,3),(1,0))
+      | ((2,2),(0,0),(4,0))
+      | ((3,3),(1,1),(3,0))
       | ((0,0))
       | ((0,1),(0,1))
 (4 rows)
 
 -- overlap
 SELECT '' AS three, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 && '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 && '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  three |         f1          
 -------+---------------------
-       | ((2,0),(2,4),(0,0))
-       | ((3,1),(3,3),(1,0))
+       | ((2,2),(0,0),(4,0))
+       | ((3,3),(1,1),(3,0))
 (2 rows)
 
 -- left overlap
 SELECT '' AS four, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 &< '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 &< '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  four |         f1          
 ------+---------------------
-      | ((2,0),(2,4),(0,0))
-      | ((3,1),(3,3),(1,0))
+      | ((3,3),(1,1),(3,0))
       | ((0,0))
       | ((0,1),(0,1))
-(4 rows)
+(3 rows)
 
 -- right overlap
 SELECT '' AS two, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 &> '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 &> '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  two |         f1          
 -----+---------------------
-     | ((3,1),(3,3),(1,0))
+     | ((3,3),(1,1),(3,0))
 (1 row)
 
 -- left of
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 << '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 << '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  one |      f1       
 -----+---------------
      | ((0,0))
      | ((0,1),(0,1))
 (2 rows)
 
 -- right of
 SELECT '' AS zero, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 >> '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 >> '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  zero | f1 
 ------+----
 (0 rows)
 
 -- contained
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 <@ polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 <@ polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  one |         f1          
 -----+---------------------
-     | ((3,1),(3,3),(1,0))
+     | ((3,3),(1,1),(3,0))
 (1 row)
 
 -- same
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 ~= polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 ~= polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  one |         f1          
 -----+---------------------
-     | ((3,1),(3,3),(1,0))
+     | ((3,3),(1,1),(3,0))
 (1 row)
 
 -- contains
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 @> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 @> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
  one |         f1          
 -----+---------------------
-     | ((3,1),(3,3),(1,0))
+     | ((3,3),(1,1),(3,0))
 (1 row)
 
 --
 -- polygon logic
 --
 -- 3	      o
---	      |
--- 2	    + |
---	   /  |
+--		     /|
+-- 2		+ |
+--		   /  |
 -- 1	  / o +
 --       /    |
 -- 0	+-----o-+
 --
---	0 1 2 3 4
+--		0 1 2 3 4
 --
 -- left of
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' << polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' << polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
  false 
 -------
  f
 (1 row)
 
 -- left overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' << polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS true;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' << polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS true;
  true 
 ------
  f
 (1 row)
 
 -- right overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' &> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' &> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
  false 
 -------
  f
 (1 row)
 
 -- right of
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' >> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' >> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
  false 
 -------
  f
 (1 row)
 
 -- contained in
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' <@ polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' <@ polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
  false 
 -------
  f
 (1 row)
 
 -- contains
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' @> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' @> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
  false 
 -------
  f
 (1 row)
 
+-- same
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' ~= polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
+ false 
+-------
+ f
+(1 row)
+
+-- overlap
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' && polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS true;
+ true 
+------
+ t
+(1 row)
+
 --     +------------------------+
 --     |    *---*               1
 --     |  + |   |
 --     |  2 *---*
 --     +------------------------+
 --                              3
 --     endpoints '+' is ofr one polygon, '*' - for another
 --     Edges 1-2, 2-3 are not shown on picture
 SELECT '((0,4),(6,4),(1,2),(6,0),(0,0))'::polygon @> '((2,1),(2,3),(3,3),(3,1))'::polygon AS "false";
  false 
@@ -226,34 +239,20 @@ SELECT '((1,1),(1,4),(5,4),(5,3),(2,3),(2,2),(5,2),(5,1))'::polygon @> '((3,2),(
 --     |    |    |
 --     |    *----*
 --     |         |
 --     +---------+
 SELECT '((0,0),(0,3),(3,3),(3,0))'::polygon @> '((2,1),(2,2),(3,2),(3,1))'::polygon AS "true";
  true 
 ------
  t
 (1 row)
 
--- same
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' ~= polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
- false 
--------
- f
-(1 row)
-
--- overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' && polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS true;
- true 
-------
- t
-(1 row)
-
 --     +--------------------+
 --     |    *---*       	1
 --     |  + |   |
 --     |  2 *---*
 --     +--------------------+
 --                      	3
 --     Edges 1-2, 2-3 are not shown on picture
 SELECT '((0,4),(6,4),(1,2),(6,0),(0,0))'::polygon && '((2,1),(2,3),(3,3),(3,1))'::polygon AS "true";
  true 
 ------
diff --git a/src/test/regress/sql/polygon.sql b/src/test/regress/sql/polygon.sql
index 2dad566..1468b66 100644
--- a/src/test/regress/sql/polygon.sql
+++ b/src/test/regress/sql/polygon.sql
@@ -1,32 +1,32 @@
 --
 -- POLYGON
 --
 -- polygon logic
 --
 -- 3	      o
---	      |
+--		      |
 -- 2	    + |
---	   /  |
--- 1	  # o +
---       /    |
+--		   /  |
+-- 1	  #   +
+--       /  o |
 -- 0	#-----o-+
 --
---	0 1 2 3 4
+--		0 1 2 3 4
 --
 
 CREATE TABLE POLYGON_TBL(f1 polygon);
 
 
-INSERT INTO POLYGON_TBL(f1) VALUES ('(2.0,0.0),(2.0,4.0),(0.0,0.0)');
+INSERT INTO POLYGON_TBL(f1) VALUES ('(2.0,2.0),(0.0,0.0),(4.0,0.0)');
 
-INSERT INTO POLYGON_TBL(f1) VALUES ('(3.0,1.0),(3.0,3.0),(1.0,0.0)');
+INSERT INTO POLYGON_TBL(f1) VALUES ('(3.0,3.0),(1.0,1.0),(3.0,0.0)');
 
 -- degenerate polygons
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0,0.0)');
 
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0,1.0),(0.0,1.0)');
 
 -- bad polygon input strings
 INSERT INTO POLYGON_TBL(f1) VALUES ('0.0');
 
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0.0 0.0');
@@ -36,87 +36,93 @@ INSERT INTO POLYGON_TBL(f1) VALUES ('(0,1,2)');
 INSERT INTO POLYGON_TBL(f1) VALUES ('(0,1,2,3');
 
 INSERT INTO POLYGON_TBL(f1) VALUES ('asdf');
 
 
 SELECT '' AS four, * FROM POLYGON_TBL;
 
 -- overlap
 SELECT '' AS three, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 && '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 && '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- left overlap
 SELECT '' AS four, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 &< '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 &< '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- right overlap
 SELECT '' AS two, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 &> '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 &> '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- left of
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 << '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 << '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- right of
 SELECT '' AS zero, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 >> '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 >> '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- contained
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 <@ polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 <@ polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- same
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 ~= polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 ~= polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 -- contains
 SELECT '' AS one, p.*
    FROM POLYGON_TBL p
-   WHERE p.f1 @> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)';
+   WHERE p.f1 @> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)';
 
 --
 -- polygon logic
 --
 -- 3	      o
---	      |
--- 2	    + |
---	   /  |
+--		     /|
+-- 2		+ |
+--		   /  |
 -- 1	  / o +
 --       /    |
 -- 0	+-----o-+
 --
---	0 1 2 3 4
+--		0 1 2 3 4
 --
 -- left of
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' << polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' << polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
 
 -- left overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' << polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS true;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' << polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS true;
 
 -- right overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' &> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' &> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
 
 -- right of
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' >> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' >> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
 
 -- contained in
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' <@ polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' <@ polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
 
 -- contains
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' @> polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' @> polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
+
+-- same
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' ~= polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS false;
+
+-- overlap
+SELECT polygon '(2.0,2.0),(0.0,0.0),(4.0,0.0)' && polygon '(3.0,3.0),(1.0,1.0),(3.0,0.0)' AS true;
 
 --     +------------------------+
 --     |    *---*               1
 --     |  + |   |
 --     |  2 *---*
 --     +------------------------+
 --                              3
 --     endpoints '+' is ofr one polygon, '*' - for another
 --     Edges 1-2, 2-3 are not shown on picture
 SELECT '((0,4),(6,4),(1,2),(6,0),(0,0))'::polygon @> '((2,1),(2,3),(3,3),(3,1))'::polygon AS "false";
@@ -141,26 +147,20 @@ SELECT '((1,1),(1,4),(5,4),(5,3),(2,3),(2,2),(5,2),(5,1))'::polygon @> '((3,2),(
 
 --     +---------+
 --     |         |
 --     |    *----*
 --     |    |    |
 --     |    *----*
 --     |         |
 --     +---------+
 SELECT '((0,0),(0,3),(3,3),(3,0))'::polygon @> '((2,1),(2,2),(3,2),(3,1))'::polygon AS "true";
 
--- same
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' ~= polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS false;
-
--- overlap
-SELECT polygon '(2.0,0.0),(2.0,4.0),(0.0,0.0)' && polygon '(3.0,1.0),(3.0,3.0),(1.0,0.0)' AS true;
-
 --     +--------------------+
 --     |    *---*       	1
 --     |  + |   |
 --     |  2 *---*
 --     +--------------------+
 --                      	3
 --     Edges 1-2, 2-3 are not shown on picture
 SELECT '((0,4),(6,4),(1,2),(6,0),(0,0))'::polygon && '((2,1),(2,3),(3,3),(3,1))'::polygon AS "true";
 
 --     +--+ *--*
#2Robert Haas
robertmhaas@gmail.com
In reply to: Emre Hasegeli (#1)
Re: Shapes on the regression test for polygon

On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli <emre@hasegeli.com> wrote:

The first two shapes on src/test/regress/sql/polygon.sql do not make
sense to me. They look more like polygons with some more tabs,
but still did not match the coordinates. I changed them to make
consistent with the shapes. I believe this was the intention of
the original author. Patch attached.

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well. Either way, I have no idea what the picture is
supposed to mean, because it looks to me like the original picture has
circles at (3,3) and (2,1) and plus signs at (2,2), (3,1), and (4,0).
With your patch applied, the circle at (2,1) has moved to (2,0.5). I
can't correlate any of this to the test cases you modified (and which
I see no reason to modify, whether they match the picture or not).

And really, if the diagram is confusing, let's just remove it. We
don't really need our regression test comments to illustrate what a
triangle looks like, or how to do bad ASCII art.

Personally, though, my vote would be to just leave it the way it is.
I don't think there's really a problem here in need of solving.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#2)
Re: Shapes on the regression test for polygon

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli <emre@hasegeli.com> wrote:

The first two shapes on src/test/regress/sql/polygon.sql do not make
sense to me.

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well.

Perhaps we should expand tabs to spaces in those ascii-art diagrams?

Personally, though, my vote would be to just leave it the way it is.
I don't think there's really a problem here in need of solving.

I concur with doubting that changing the actual regression test cases
is a good thing to do, at least not without more analysis. But "the
pictures make no sense with the wrong tab setting" is a pretty simple
issue, and one that I can see biting people.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#3)
Re: Shapes on the regression test for polygon

On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli <emre@hasegeli.com> wrote:

The first two shapes on src/test/regress/sql/polygon.sql do not make
sense to me.

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well.

Perhaps we should expand tabs to spaces in those ascii-art diagrams?

Personally, though, my vote would be to just leave it the way it is.
I don't think there's really a problem here in need of solving.

I concur with doubting that changing the actual regression test cases
is a good thing to do, at least not without more analysis. But "the
pictures make no sense with the wrong tab setting" is a pretty simple
issue, and one that I can see biting people.

AFAICT, the pictures make no sense with the right tab setting, either.
The possibility that someone might use the wrong tab setting is just
icing on the cake.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Emre Hasegeli
emre@hasegeli.com
In reply to: Robert Haas (#2)
Re: Shapes on the regression test for polygon

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well. Either way, I have no idea what the picture is
supposed to mean, because it looks to me like the original picture has
circles at (3,3) and (2,1) and plus signs at (2,2), (3,1), and (4,0).
With your patch applied, the circle at (2,1) has moved to (2,0.5). I
can't correlate any of this to the test cases you modified (and which
I see no reason to modify, whether they match the picture or not).

4 space tab-stop is not the project standard?

The circle I moved does not represent a corner of the polygon. I just
moved it to make the line straight after the tabs.

And really, if the diagram is confusing, let's just remove it. We
don't really need our regression test comments to illustrate what a
triangle looks like, or how to do bad ASCII art.

Personally, though, my vote would be to just leave it the way it is.
I don't think there's really a problem here in need of solving.

I am sorry for taking your time for such a low priority problem, but
as we stumble across this, let me to make the change more clear.
According to git blame the tests with the shapes added by 04688df6.
The coordinates was written in the old format like this:

(3.0,3.0,1.0,1.0,3.0,0.0)

These are changed by 3d9584c9 to the new format like this:

(3.0,1.0),(3.0,3.0),(1.0,0.0)

In my opinion, the real intention of the first commit was to write
them in the new format like this:

(3.0,3.0),(1.0,1.0),(3.0,0.0)

It makes sense because the corners match the shape. So, I changed it
that way. If we will not going to change the tests, It would be okay
to just remove the shapes. I do not think they add more value to
the tests.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Emre Hasegeli (#5)
Re: Shapes on the regression test for polygon

Emre Hasegeli <emre@hasegeli.com> writes:

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well.

4 space tab-stop is not the project standard?

It is for C code, but there's less agreement about non-code files
(and no enforcement mechanism like pgindent, either). People may or
may not have their editors configured to do the right thing in non-C
files, so it seems best to avoid cases where it'd matter. We actually
have a policy against using tabs at all in SGML files, for example.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#4)
Re: Shapes on the regression test for polygon

On Wed, Jul 23, 2014 at 06:12:59PM -0400, Robert Haas wrote:

On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli <emre@hasegeli.com> wrote:

The first two shapes on src/test/regress/sql/polygon.sql do not make
sense to me.

Well, I think the number of tabs that makes them look best depends on
your tab-stop setting. At present, I find that with 8-space tabs
things seem to line up pretty well, whereas with your patch, 4-space
tabs line up well.

Perhaps we should expand tabs to spaces in those ascii-art diagrams?

Personally, though, my vote would be to just leave it the way it is.
I don't think there's really a problem here in need of solving.

I concur with doubting that changing the actual regression test cases
is a good thing to do, at least not without more analysis. But "the
pictures make no sense with the wrong tab setting" is a pretty simple
issue, and one that I can see biting people.

AFAICT, the pictures make no sense with the right tab setting, either.
The possibility that someone might use the wrong tab setting is just
icing on the cake.

I extracted Emre's diagram adjustments from the patch and applied it,
and no tabs now. Emre, I assume your regression changes did not affect
the diagram contents.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Emre Hasegeli
emre@hasegeli.com
In reply to: Bruce Momjian (#7)
Re: Shapes on the regression test for polygon

I extracted Emre's diagram adjustments from the patch and applied it,
and no tabs now. Emre, I assume your regression changes did not affect
the diagram contents.

Thank you for looking at it. I wanted to make the tests consistent
with the diagrams. Now they look better but they still don't make
sense with the tests. I looked at it some more, and come to the
conclusion that removing them is better than changing the tests.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Bruce Momjian
bruce@momjian.us
In reply to: Emre Hasegeli (#8)
Re: Shapes on the regression test for polygon

On Tue, Oct 14, 2014 at 05:40:06PM +0300, Emre Hasegeli wrote:

I extracted Emre's diagram adjustments from the patch and applied it,
and no tabs now. Emre, I assume your regression changes did not affect
the diagram contents.

Thank you for looking at it. I wanted to make the tests consistent
with the diagrams. Now they look better but they still don't make
sense with the tests. I looked at it some more, and come to the
conclusion that removing them is better than changing the tests.

OK, unless I hear more feedback I will remove the diagrams.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#9)
Re: Shapes on the regression test for polygon

On Tue, Oct 14, 2014 at 11:00:47AM -0400, Bruce Momjian wrote:

On Tue, Oct 14, 2014 at 05:40:06PM +0300, Emre Hasegeli wrote:

I extracted Emre's diagram adjustments from the patch and applied it,
and no tabs now. Emre, I assume your regression changes did not affect
the diagram contents.

Thank you for looking at it. I wanted to make the tests consistent
with the diagrams. Now they look better but they still don't make
sense with the tests. I looked at it some more, and come to the
conclusion that removing them is better than changing the tests.

OK, unless I hear more feedback I will remove the diagrams.

Removed.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers