Geo: Don't flip longitude of envelopes crossing dateline

[imotov]

When a envelope that crosses the dateline is specified as a part of
geo_shape query is parsed it shouldn't have its left and right points
flipped.

Fixes #34418

[elasticmachine]

Pinging @elastic/es-search-aggs

[iverase]

It is interesting to see that there was a test to actually check this coercing. I think I would like @nknize to comment here in case this coerce has been added for some use case I am not aware of. Still it seems wrong because it does not allow to build envelopes that crosses the dateline.

In the meanwhile I had a look into the documentation:

https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-shape.html#_envelope

The definition is a bit open:

Elasticsearch supports an envelope type, which consists of coordinates for upper left and lower right points of the shape to represent a bounding rectangle:

If we go for this change we should updated it and define that oder of the coordinates is important:

Elasticsearch supports an envelope type, which consists of coordinates for upper left and lower right points of the shape to represent a bounding rectangle in the format [[minLon, maxLat],[maxLon, minLat]]:

WDYT?

[iverase]

And thinking about WKT, It would be interesting to test that providing the envelope in Elasticsearch format gives the same result than when it is given in WKT.

[nknize]

Its been a while, but IIRC the coercion test was for the case where a user provides TopRight, BottomLeft instead of TopLeft, BottomRight. I think for this case the bug is: if (((lR.x < uL.x) || (uL.y < lR.y))) should instead be a logical AND: if (((lR.x < uL.x) && (uL.y < lR.y))) ? Otherwise the envelope is implied to cross the dateline?

+1 for making the doc change.

[iverase]

I don't think coerce can never work because when crossing the dateline then the envelope will be ilegal . We should remove it and change docs to make the order more explicit.

[nknize]

It should work if the logic is changed to &&? In that case the envelope crosses the dateline since upperLeft.y > lowerRight.y but upperLeft.x < lowerRight.x. Either way, I'm also fine with removing the leniency and requiring explicit order. Though I think we should note it in breaking changes in case users have relied on coerce to correct the order for them.

[iverase]

Sorry I didn't explained myself properly. It works when providing the right coordinates.
But if a user provides the bottomLeft and the upperRight coordinates, then they will be corrected except when crossing the dateline where they will get an error. That is what I would like to avoid.

[nknize]

👍 We would need to correct for that but I agree that the code starts getting far too hairy to support the leniency. +1 for requiring explicit order and documenting in breaking changes. In this case coerce should only be used for normalizing coordinates to -180 : 180 and -90 : 90.