Fix geometry_to_bing_tiles function for geometry collections #10627
Conversation
mbasmanova
force-pushed
the
fix-geometry-to-bing-tiles
branch
from
4c18ac6
to
b0653d5
Compare
| @@ -338,6 +338,11 @@ public void testGeometryToBingTiles() | |||
| assertFunction("cardinality(geometry_to_bing_tiles(ST_GeometryFromText('POLYGON ((0 0, 0 20, 20 20, 0 0))'), 14))", BIGINT, 428787L); | |||
| } | |||
|
|
|||
| private void assertGeometryToBingTiles(String wkt, int zoomLevel, List<String> expectedQuadKeys) | |||
| { | |||
| assertFunction("transform(geometry_to_bing_tiles(ST_GeometryFromText('" + wkt +"'), " + zoomLevel + "), x -> bing_tile_quadkey(x))", new ArrayType(VARCHAR), expectedQuadKeys); | |||
There was a problem hiding this comment.
Use String.format() for better readability.
| points++; | ||
| } | ||
| else { | ||
| points += ((MultiVertexGeometry) geometry).getPointCount(); |
There was a problem hiding this comment.
It can also be a Segment (Line)
| zoomLevel, | ||
| BingTile.fromCoordinates(x, y, nextZoomLevel), | ||
| blockBuilder); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static int getPointCount(Geometry geometry) | ||
| private static int getPointCount(OGCGeometry ogcGeometry) |
There was a problem hiding this comment.
This method looks like an pretty generic utility method. Do you think it is worth moving to a public utility class?
| } | ||
| } | ||
|
|
||
| return blockBuilder.build(); | ||
| } | ||
|
|
||
| private static Envelope getEnvelope(OGCGeometry ogcGeometry) |
There was a problem hiding this comment.
TIP: Think of moving getEnvelope , disjoint , contains, getPointCount , isPointOrRectangle to the GeometryUtils class. These methods (or at least some of them) seem to be pretty generic.
| @@ -256,13 +255,59 @@ public static Block geometryToBingTiles(@SqlType(GEOMETRY_TYPE_NAME) Slice input | |||
| // tile covered by the geometry. | |||
| BingTile[] tiles = getTilesInBetween(leftUpperTile, rightLowerTile, OPTIMIZED_TILING_MIN_ZOOM_LEVEL); | |||
| for (BingTile tile : tiles) { | |||
| writeTilesToBlockBuilder(geometry, zoomLevel, tile, blockBuilder); | |||
| writeTilesToBlockBuilder(ogcGeometry, zoomLevel, tile, blockBuilder); | |||
There was a problem hiding this comment.
s/writeTilesToBlockBuilder/appendIntersectingSubtiles/
| @@ -256,13 +255,59 @@ public static Block geometryToBingTiles(@SqlType(GEOMETRY_TYPE_NAME) Slice input | |||
| // tile covered by the geometry. | |||
| BingTile[] tiles = getTilesInBetween(leftUpperTile, rightLowerTile, OPTIMIZED_TILING_MIN_ZOOM_LEVEL); | |||
There was a problem hiding this comment.
@arhimondr I like your other suggestion s/writeTilesToBlockBuilder/appendIntersectingSubtiles/, but I'm not sure about this one. Here, we are not looking for intersecting tiles, but rather a set of sub-tiles covering some larger tiles. One can think about this function as 2d version of sequence(start, stop, step) where start is a tile in one corner, stop is a tile in the opposite corner and step is zoom level. Perhaps, s/getTilesInBetween/tilesSequence?
There was a problem hiding this comment.
The name of the method is fine. I was talking about the variable name. But even so, i don't have a strong opinion here.
| @@ -307,6 +307,12 @@ public void testGeometryToBingTiles() | |||
| assertGeometryToBingTiles("POLYGON ((10 10, -10 10, -20 -15, 10 10))", 3, ImmutableList.of("033", "211", "122")); | |||
| assertGeometryToBingTiles("POLYGON ((10 10, -10 10, -20 -15, 10 10))", 6, ImmutableList.of("211102", "211120", "033321", "033323", "211101", "211103", "211121", "033330", "033332", "211110", "211112", "033331", "033333", "211111", "122220", "122222", "122221")); | |||
|
|
|||
| assertGeometryToBingTiles("GEOMETRYCOLLECTION (POINT (60 30.12))", 10, ImmutableList.of("1230301230")); | |||
There was a problem hiding this comment.
Add a GEOMETRYCOLLECTION EMPTY case
mbasmanova
force-pushed
the
fix-geometry-to-bing-tiles
branch
2 times, most recently
from
aa22a5f
to
610d315
Compare
| } | ||
|
|
||
| /** | ||
| * @return true if the geometry is a point or a rectangle |
mbasmanova
force-pushed
the
fix-geometry-to-bing-tiles
branch
from
610d315
to
0450eb1
Compare
Fix NPE in geometry_to_bing_tiles when input is a geometry collection.
mbasmanova
force-pushed
the
fix-geometry-to-bing-tiles
branch
from
0450eb1
to
82014c7
Compare
Return empty array if input geometry is empty, Before, the function threw an exception.
mbasmanova
force-pushed
the
fix-geometry-to-bing-tiles
branch
from
82014c7
to
afbe767
Compare
Fix NPE in geometry_to_bing_tiles function when input is a geometry collection.
Fixes #10623