Re: [HACKERS] 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 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
Re: [HACKERS] 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 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
Re: [HACKERS] 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
Re: [HACKERS] 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 wrote: > > Robert Haas writes: > >> On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli 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 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
Re: [HACKERS] Shapes on the regression test for polygon
Emre Hasegeli 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
Re: [HACKERS] 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
Re: [HACKERS] Shapes on the regression test for polygon
On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli 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
Re: [HACKERS] Shapes on the regression test for polygon
Robert Haas writes: > On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli 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
Re: [HACKERS] Shapes on the regression test for polygon
On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli 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
[HACKERS] Shapes on the regression test for polygon
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.
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.
