Opened 7 years ago

Closed 6 years ago

#2168 closed defect (fixed)

ST_Distance is not always commutative

Reported by: realityexists Owned by: robe
Priority: medium Milestone: PostGIS 2.0.4
Component: postgis Version: trunk
Keywords: Cc: jkania

Description

I've found a case where changing the order of operands for ST_Distance(geog) causes it to return slightly different results:

SELECT ST_Distance('POINT(-40 46.167)'::geography, 'POINT(0 7.167)'::geography)
	- ST_Distance('POINT(0 7.167)'::geography, 'POINT(-40 46.167)'::geography);

returns 9.31322574615479e-010 on r10952, Windows x64, PostgreSQL 9.1.4 and 9.2.1. On 32-bit Linux it returns 0 as expected.

This caused some odd behaviour in my application, which orders sets of points by their distance from each other - the result was different depending on the order in which points were considered. There's an easy workaround, of course - just round the results - but I can imagine other developers scratching their heads over this as well.

Change History (21)

comment:1 Changed 7 years ago by robe

FWIW: Same issue on my 9.2.2 windows 2008 64-bit

9.31322574615479e-010

However my 9.2.2 32-bit windows 7 32-bit shows this

0

So seems like it might possibly be a 64-bit issue maybe because 64-bit can carry more digits (whether it is only windows 64-bit is a different matter)

You don't happen to have a 64-bit Linux you can test with?

comment:2 Changed 7 years ago by realityexists

No, I don't have a 64-bit Linux machine to test with.

comment:3 Changed 7 years ago by strk

Gives 0 here:

 POSTGIS="2.0.3SVN r10818" GEOS="3.4.0dev-CAPI-1.8.0 r3741" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.0.2SVN r10758" need upgrade) TOPOLOGY (topology procs from "2.0.3SVN r10797" need upgrade) RASTER (raster procs from "2.0.2SVN r10758" need upgrade)

 PostgreSQL 9.1.7 on x86_64-unknown-linux-gnu, compiled by gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, 64-bit

comment:4 Changed 7 years ago by robe

oops forgot to output my full:

My 64-bit windows showing the issue is

PostgreSQL 9.2.2, compiled by Visual C++ build 1600, 64-bit
POSTGIS="2.1.0SVN r10851" GEOS="3.4.0dev-CAPI-1.8.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.1.0SVN r10798" need upgrade) TOPOLOGY RASTER (raster procs from "2.1.0SVN r10798" need upgrade)

My 32-bit workstation not showing the issue is this

PostgreSQL 9.2.1, compiled by Visual C++ build 1600, 32-bit
POSTGIS="2.1.0SVN r10868" GEOS="3.4.0dev-CAPI-1.8.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08 GDAL_DATA not found" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

strk you have a 2.1 to test with as well? I haven't tried 2.0 branch yet and I'm living dangerously running with 2.1 in production.

comment:5 Changed 7 years ago by robe

nevermind just tested on my same 64-bit instance with some 2.0.2SVN database I had lying around and got the same issue.

comment:6 Changed 7 years ago by strk

I still get 0 here:

 POSTGIS="2.1.0SVN r10816" GEOS="3.4.0dev-CAPI-1.8.0 r3741" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.1.0SVN r10796" need upgrade) TOPOLOGY (topology procs from "2.1.0SVN r10807" need upgrade) RASTER (raster procs from "2.1.0SVN r10796" need upgrade)

Same machine and same postgresql version

comment:7 Changed 7 years ago by robe

Milestone: PostGIS 2.1.0PostGIS 2.0.4

comment:8 Changed 7 years ago by jkania

Cc: jkania added

I get 0 for that query but for this one:

SELECT ST_Distance('POINT(18.5107234 54.7587757)'::geography, 'POINT(18.58218 54.7344227)'::geography)
	- ST_Distance('POINT(18.58218 54.7344227)'::geography, 'POINT(18.5107234 54.7587757)'::geography);

I get 1.18234311230481e-010.

"POSTGIS="2.0.3 r11132" GEOS="3.3.8-CAPI-1.7.8" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER"
"PostgreSQL 9.2.3, compiled by Visual C++ build 1600, 32-bit"

Windows 7 32-bit.

comment:9 Changed 7 years ago by robe

My 32-bit PostgreSQL 9.2 on my windows 7 64-bit behaves like yours (0 for first and 1.18234311230481e-010 for second)

Windows 2008 and windows 7 64-bit 9.2 64-bit

9.31322574615479e-010 for the first
1.18234311230481e-010 for the second
--just to test constancy
SELECT ST_Distance(g1,g2) As dist_g1_g2, ST_Distance(g2,g1) AS dist_g2_g1
FROM (SELECT 'POINT(18.5107234 54.7587757)'::geography As g1, 'POINT(18.58218 54.7344227)'::geography As g2) As a;

   dist_g1_g2    |    dist_g2_g1
-----------------+------------------
 5340.7623739472 | 5340.76237394708

I guess seems to only happen on Windows. Paul did you say you are going to just bite the bullet and round or is this nanometer issue something we should be worried about and the fact only iwnodws exhibits it.

comment:10 Changed 6 years ago by pramsey

Resolution: fixed
Status: newclosed

Fixed in 2.0 at r11529 and trunk at r11530

comment:11 Changed 6 years ago by strk

Resolution: fixed
Status: closedreopened

I don't like this approach. Better to figure out why distance_spheroid is returning a different result. Beside, I see that distance_spheroid takes a tolerance already. Too many tolerances handled in too many places for my taste.

Can we come up with a cunit test for this case ? No fixes should be committed in 2.0 with no unit tests.

comment:12 Changed 6 years ago by pramsey

Sandro, If you're -1 on the patch, please back it out so we can retain the non-commutative behaviour: -1 implies a willingness to resolve the problem in the "proper way".

comment:13 Changed 6 years ago by pramsey

In support of my approach: the problem occurs on only one platform/architecture combination, and only at the nanometer level. It bugs other people enough to complain, but doesn't bug me enough to track all tolerance logic through the whole geodetic tree, so I applied a bandaid rather than a global policy.

comment:14 Changed 6 years ago by robe

+1 for keeping the bandaid for now. Sandro unless you are about to start testing on windows or provide a test and fix that doesn't make winnie scream, I say leave well enough alone :)

comment:15 Changed 6 years ago by robe

Resolution: fixed
Status: reopenedclosed

comment:16 Changed 6 years ago by strk

Resolution: fixed
Status: closedreopened

It still lacks a testcase, don't you think ?

comment:17 Changed 6 years ago by robe

You mean just add this ticket to example to tickets. Yah I guess we can do that much. Alright I'll take it upon myself to add this example since I'm the only one here that can replicate the issue.

But you think we should just throw it in tickets.sql or you want it in distance. I'm tempted to just throw it in tickets where we put the other ticketed ones.

comment:18 Changed 6 years ago by robe

Owner: changed from pramsey to robe
Status: reopenednew

comment:19 Changed 6 years ago by strk

tickets.sql is fine to me

comment:20 Changed 6 years ago by robe

uh why did I agree to this. Oh well will try to get to it in next day or so.

comment:21 Changed 6 years ago by robe

Resolution: fixed
Status: newclosed

put in regress example at r11919 for 2.0, r11920 for 2.2, and r11918 for 2.1

Note: See TracTickets for help on using tickets.