ST_3DClosestPoint, ST_3DLongestLine, ST_3DMaxDistance regression between 2.1.8 and 2.2.0rc1

[robe]

— tsting

POSTGIS="2.1.8 r13780" GEOS="3.5.0-CAPI-1.9.0 r4088" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.11.1, released 2014/09/24" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

POSTGIS="2.2.0dev r14092" GEOS="3.5.0-CAPI-1.9.0 r4088" PROJ="Rel. 4.9.1, 04 March 2015" GDAL="GDAL 2.0.1, released 2015/09/15" LIBXML="2.7.8" LIBJSON="0.12" RASTER

On both 2.2.0 and 2.1.8 this works — The test

SELECT ST_AsText(ST_3DClosestPoint('SRID=4326;PolyhedralSurface( 
((0 0 0, 0 0 1, 0 1 1, 0 1 0, 0 0 0)),  
((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0)), ((0 0 0, 1 0 0, 1 0 1, 0 0 1, 0 0 0)),  ((1 1 0, 1 1 1, 1 0 1, 1 0 0, 1 1 0)),  
((0 1 0, 0 1 1, 1 1 1, 1 1 0, 0 1 0)),  ((0 0 1, 1 0 1, 1 1 1, 0 1 1, 0 0 1)) 
)'::geometry, 

'SRID=4326;PolyhedralSurface( 
((0 0 0, 0 0 1, 0 1 1, 0 1 0, 0 0 0)),  
((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0)) )'::geometry ))

and gives

POINT Z (0 0 0)

However this test runs in 2.1.8 and throws an error in 2.2

SELECT ST_AsText(ST_3DClosestPoint('SRID=4326;PolyhedralSurface( 
((0 0 0, 0 0 1, 0 1 1, 0 1 0, 0 0 0)),  
((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0)), ((0 0 0, 1 0 0, 1 0 1, 0 0 1, 0 0 0)),  ((1 1 0, 1 1 1, 1 0 1, 1 0 0, 1 1 0)),  
((0 1 0, 0 1 1, 1 1 1, 1 1 0, 0 1 0)),  ((0 0 1, 1 0 1, 1 1 1, 0 1 1, 0 0 1)) 
)'::geometry, 

'SRID=4326;POINT(0 0)'::geometry ))

— in 2.1.8 this gives -

POINT Z (0 0 0)

— in 2.2.0 it throws an error

ERROR:  Unsupported geometry type: PolyhedralSurface

However:

SELECT ST_AsText(ST_3DClosestPoint('SRID=4326;PolyhedralSurface( 
((0 0 0, 0 0 1, 0 1 1, 0 1 0, 0 0 0)),  
((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0)), ((0 0 0, 1 0 0, 1 0 1, 0 0 1, 0 0 0)),  ((1 1 0, 1 1 1, 1 0 1, 1 0 0, 1 1 0)),  
((0 1 0, 0 1 1, 1 1 1, 1 1 0, 0 1 0)),  ((0 0 1, 1 0 1, 1 1 1, 0 1 1, 0 0 1)) 
)'::geometry, 

'SRID=4326;POINT(0 0 0)'::geometry ));

works and give the

POINT Z (0 0 0)

Is this an intentional change? I know we went back and forth on what do do with 2d, but forgot where the ball fell.

Anyway regardless the message is wrong since 2 polyhedrals work and a 3D point and 3d polyhedral work

[robe]

These may just require change in error messages and a breaking change notice.

The issue is not just limited to polyhedralsurfaces. I thought it was maybe any 2D, but while this:

fails (ST_3DLongestLine fails as well) — works in 2.1.8

SELECT ST_3DClosestPoint('LINESTRING(-11.1111111 55,55 -11.1111111)'::geometry,'MULTILINESTRING((-10 40,40 -10),(-10 55,55 -10),(-10 70,70 -10),(0 40,40 0),(0 55,55 0),(0 70,70 0),(10 40,40 10),(10 55,55 10),(10 70,70 10),(20 40,40 20),(20 55,55 20),(20 70,70 20),(30 40,40 30),(30 55,55 30),(30 70,70 30),(40 55,55 40),(40 70,70 40),(50 40,40 50),(50 55,55 50),(50 70,70 50))'::geometry)

ERROR:  Unsupported geometry type: MultiLineString

This works:

SELECT ST_3DLongestLine('LINESTRING(1 2, 3 4)'::geometry, 'LINESTRING(1 2, 3 5)'::geometry);

This fails:

SELECT ST_3DLongestLine('LINESTRING(1 2, 3 4)'::geometry, ST_Multi('LINESTRING(1 2, 3 5)'::geometry))

Why whether a string is multi or not I don't know why that would make a difference.

[nicklas]

I think this is side effects of my handling of mixed dimmension.

I will look at it.

[robe]

Also check ST_3DWithin, ST_3DIntersects, ST_3DDFullyWithin (those are failing too) — I'm guessing all these regession failures are coming from the same root.

[nicklas]

Ok, this is a little scary.

When dealing with mixed dimensionality in #2034 I put check to not let multi-geometries through.

But now I cannot see the problem to just let them go through. It seems to work as expected, and I have an idea about why I was afraid of it.

But I think and hope that I was wrong then because not that check is removed.

Please test this. What it is about is different combinations of multi/multi, multi/single in ST_3DClosestPoint and ST_3DShortestLine

The problem is that it is so hard to verify if the answer is right more than in the most simple examples.

commit in r14179

[robe]

okay I'll rerun my tests and report back. thanks

[robe]

Only thing left is a failure with tIN And empty geometries that used to return something in 2.1. Not sure why that ever returned anything anyway and was only for the useless case of TIN and empty so we can ignore that.

I am also seeing differences in answers between 2.1.8 and 2.2.0 which I am guessing are intentional. I think we just need to flag in the docs these functions changed behavior in case of 2D/3D, 2D/2D, 2d/2DM etc

Example:

SELECT ST_AsText(ST_3DClosestPoint('POINT(-11.1111111 40)'::geometry, 'POINT(-11.1111111 40)'::geometry));

• 2.1.8 answer

POINT Z (-11.1111111 40 0)

— 2.2.0 answer

POINT(-11.1111111 40)

{{{ — 2d and 3d line SELECT ST_AsText(ST_3DClosestPoint('LINESTRING(-11.1111111 70,70 -11.1111111)'::geometry, 'LINESTRING Z (-10 40 1,-9 41 1)'::geometry )); }}}

-- 2.1.8 answer
POINT Z (4.44444445 54.44444445 0)

-- 2.2.0 answer

POINT Z (4.44444445 54.44444445 1)

Is it okay for me to just throw a changed notice in the docs for all these functions something the effect

---

Changed 2.2.0: In case of 2 2d geometries the function will return a 2d geometry instead of 3D with 0 for Z

In case of 2d and 3d - the function will consider the 2D geometry as having a Z from 0 to infinity

---

[nicklas]

The tin/empty thing I think was a missing check in 2.1, if I recall right.

The change in answer is the result of #2034.

But your first example might reveal a problem. Is it right to return 2d from a 3d function? I think we have said before that the answer shall be 3d. But then, what z-value to use? the result of #2034 is about that unknown is not 0. so 0 shouldn't be the answer. Is NULL an option or is 2d the best way to go?

What does ST_force3d put in z-coord whith 2d as input?

[robe]

Yah I was thinking about that. There is no good z to use. I think the only thing we could do is return same answer as 2D (whic is what you are doing) or return an error (which assumes the user accidentally used 3D functon when they wanted 2D). I would just leave it as is.

[robe]

ST_Force3D puts in a 0.

[nicklas]

To make it consistent with ST_Force3D the easiest would be to wrap the functions into ST_Force3D I guess. But the question is if that is what we

want

[robe]

no. Lets keep it the way it is. There is no right answer and changing code just to get old answer is not worth it.

[strk]

Regina 0 to Infinity is half true. Guess it is -Infinity to Infinity…

[strk]

ST_Force3D adding 0 is not nice, btw. There's an enhancement ticket asking to be able to specify a value instead: #3057

[robe]

I stick with lets do nothing and per chance someone complains it's spelled out in docs as a breaking change and they can put the force3d in their code if they want it.

all in agreement +1.

[robe]

Okay just noted it in docs at r14183. I figure can sort out the details if they think they are affected by the change.

[nicklas]

Thanks a lot Regina