Skip to content

Commit 38436f6

Browse files
kevintang2022kevintang2022
authored
fix(geospatial): raise the bing tile zoom-span limit as a user error (facebookincubator#18993)
Summary: `geometry_to_bing_tiles` surfaces the parent-to-child zoom span limit through `VELOX_FAIL`, so it arrives as a runtime error and `try()` cannot catch it. The same limit, reached through the `bing_tile_children` UDF, is returned as `Status::UserError` and is catchable. `BingTileType::bingTileChildren` returns `folly::Expected` with a message rather than throwing, leaving classification to each caller. The two callers disagree. The condition depends only on the input geometry and the requested zoom, both user supplied, and the adjacent covering-count guard in the same function already uses `VELOX_USER_FAIL`. A query that wraps the call in `try()` therefore dies outright instead of dropping the offending row, which is what the same query does on Presto Java. Differential Revision: D119884072 Pull Request resolved: facebookincubator#18993
1 parent 17fbf01 commit 38436f6

2 files changed

Lines changed: 18 additions & 2 deletions

File tree

‎velox/functions/prestosql/geospatial/GeometryUtils.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ std::vector<int64_t> getRawTilesCoveringGeometry(
432432
auto children = BingTileType::bingTileChildren(
433433
entry.tile, BingTileType::bingTileZoom(entry.tile) + 1, 1);
434434
if (FOLLY_UNLIKELY(children.hasError())) {
435-
VELOX_FAIL(children.error());
435+
VELOX_USER_FAIL(children.error());
436436
}
437437
std::for_each(
438438
children.value().begin(), children.value().end(), addIntersecting);
@@ -460,7 +460,7 @@ std::vector<int64_t> getMinimalTilesCoveringGeometry(
460460
auto expectedChildren =
461461
BingTileType::bingTileChildren(thisTile, zoom, maxZoomShift);
462462
if (FOLLY_UNLIKELY(expectedChildren.hasError())) {
463-
VELOX_FAIL(expectedChildren.error());
463+
VELOX_USER_FAIL(expectedChildren.error());
464464
}
465465
outputTiles.insert(
466466
outputTiles.end(),

‎velox/functions/prestosql/tests/GeometryFunctionsTest.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3516,6 +3516,22 @@ TEST_F(GeometryFunctionsTest, testGeometryToBingTiles) {
35163516
"cardinality(geometry_to_bing_tiles(ST_GeometryFromText(c0), c1))",
35173517
std::optional<std::string>("POLYGON ((0 0, 0 20, 20 20, 0 0))"),
35183518
std::optional<int32_t>(14)));
3519+
3520+
// The parent-to-child zoom span limit depends on the input geometry and the
3521+
// requested zoom, so it is a user error and must be catchable by try(). The
3522+
// same limit reached through bing_tile_children already reports a user error.
3523+
{
3524+
queryCtx_->testingOverrideConfigUnsafe(
3525+
{{core::QueryConfig::kDebugBingTileChildrenMaxZoomShift, "2"}});
3526+
VELOX_ASSERT_USER_THROW(
3527+
evaluateOnce<int64_t>(
3528+
"cardinality(geometry_to_bing_tiles(ST_GeometryFromText(c0), c1))",
3529+
std::optional<std::string>("POLYGON ((0 0, 0 20, 20 20, 0 0))"),
3530+
std::optional<int32_t>(14)),
3531+
"Difference between parent zoom");
3532+
queryCtx_->testingOverrideConfigUnsafe(
3533+
{{core::QueryConfig::kDebugBingTileChildrenMaxZoomShift, "7"}});
3534+
}
35193535
}
35203536

35213537
TEST_F(GeometryFunctionsTest, testGeometryToDissolvedBingTiles) {

0 commit comments

Comments
 (0)