line_perp() (?-|) is broken. - Mailing list pgsql-hackers

From Kyotaro HORIGUCHI
Subject line_perp() (?-|) is broken.
Date
Msg-id 20180201.205138.34583581.horiguchi.kyotaro@lab.ntt.co.jp
Whole thread Raw
Responses Re: line_perp() (?-|) is broken.  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I happend to see a strange geometric calcualtion on master/HEAD.

CREATE TABLE t (l1 line, l2 line);
INSERT INTO t
  (SELECT line(point(0, 0), point(x, y)), line(point(0,0), point(-y, x))
   FROM (SELECT random() x, random() y FROM generate_series(0, 1000)) AS a);
SELECT l1?-|l2 AS is_perp, l1, l2 FROM t;
 is_perp |             l1             |             l2              
---------+----------------------------+-----------------------------
 f       | {0.215980373614968,-1,0}   | {-4.63005032940037,-1,0}
 f       | {1.51653638154567,-1,0}    | {-0.659397303070824,-1,0}
 f       | {0.596861744826871,-1,0}   | {-1.67542987746696,-1,0}
...
 SELECT l1?-|l2 AS is_perp, l1, l2 FROM t WHERE l1?-|l2;
 is_perp | l1 | l2 
---------+----+----
(0 rows)

The two lines in a row are always perpendicular, but ?-| doesn't
agree.

line_perp() is working with the following arithmetic.

  (l1->A * l2->B) / (l1->B * l2->A) ==  -1.0

or 

  (l1->A * l2->B) + (l1->B * l2->A) ==  0

If the plus were a minus, it would calculate the cross product of
the two vectors and tell if the two lines are "parallel" or
not... Anyway it doesn't work as expected. At least back to 9.0
has the same expression in the function.

Instead, calculating inner product of the two direction vectors
works as expected.

  (l1->A * l2->A) + (l1->B * l2->B) == 0

FPzero checks at the beggining of the function for the purpose of
avoiding div0 is useless with this form of expression.


With the attached patch, we will have the correct result.

 is_perp |             l1             |             l2              
---------+----------------------------+-----------------------------
 t       | {0.215980373614968,-1,0}   | {-4.63005032940037,-1,0}
 t       | {1.51653638154567,-1,0}    | {-0.659397303070824,-1,0}
 t       | {0.596861744826871,-1,0}   | {-1.67542987746696,-1,0}
 t       | {2.14739225724798,-1,0}    | {-0.465681105361517,-1,0}
...
SELECT l1?-|l2 AS is_perp, l1, l2 FROM t WHERE NOT l1?-|l2;
 is_perp | l1 | l2 
---------+----+----
(0 rows)

regards.

diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c
index f57380a..3b12d41 100644
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -1119,12 +1119,7 @@ line_perp(PG_FUNCTION_ARGS)
     LINE       *l1 = PG_GETARG_LINE_P(0);
     LINE       *l2 = PG_GETARG_LINE_P(1);
 
-    if (FPzero(l1->A))
-        PG_RETURN_BOOL(FPzero(l2->B));
-    else if (FPzero(l1->B))
-        PG_RETURN_BOOL(FPzero(l2->A));
-
-    PG_RETURN_BOOL(FPeq(((l1->A * l2->B) / (l1->B * l2->A)), -1.0));
+    PG_RETURN_BOOL(FPzero(l1->A * l2->A + l1->B * l2->B));
 }
 
 Datum

pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: [HACKERS] MERGE SQL Statement for PG11
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: [HACKERS] [PATCH] Improve geometric types