feat(vortex-geo): geometry Bbox zone-map statistic + distance-filter pruning#8646
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
HarukiMoriarty
force-pushed
the
nemo/geo-bounds-zonemap
branch
3 times, most recently
from
4274729 to
bda1557
Compare
| /// The default per-chunk zone statistic to store for a column of `input_dtype`, or `None` if | ||
| /// this aggregate isn't one. |
There was a problem hiding this comment.
Nit: this is kind of hard to understand, maybe say something like this or similar:
/// The default zone statistic (per-chunk) for a column of `input_dtype`,
/// or `None` if the dtype is not supported.
```rust
connortsui20
left a comment
connortsui20
left a comment
There was a problem hiding this comment.
Seems fine to me, but someone who has worked on aggregates and stats more should probably review this
…-filter pruning Signed-off-by: Nemo Yu <zyu379@wisc.edu>
HarukiMoriarty
force-pushed
the
nemo/geo-bounds-zonemap
branch
from
bda1557 to
119fe60
Compare
| let Some(radius) = expr.child(1).as_opt::<Literal>() else { | ||
| return Ok(None); | ||
| }; | ||
| let Ok(radius) = f64::try_from(radius) else { |
There was a problem hiding this comment.
is this a bug? hitting this case
There was a problem hiding this comment.
- A null literal (distance <= NULL), no value to reason about. Scanning is sound: the filter evaluates to null at scan time and drops every row anyway; we just missed a chance to skip the chunk early.
- A non-primitive literal, mostly impossible in a typechecked plan (comparing f64 to a string fails planning), except extension-typed literals, which Binary's typecheck deliberately lets through.
There was a problem hiding this comment.
Did you test this I am not how f64::try_from and Option::<f64>::try_from differ?
There was a problem hiding this comment.
Also can you ever have a value that is not in fact f64 for a value typechecked pr
There was a problem hiding this comment.
Did you test this I am not how
f64::try_fromandOption::<f64>::try_fromdiffer?
They differ on both axes: f64::try_from casts any primitive ptype (integer radius → 10.0), errors on null/non-primitive; Option::<f64>::try_from requires exactly F64 (an integer literal errors, no cast), null → Ok(None). So the Option form would stop integer radii from pruning. Wasn't tested before, I added tests to pin what we tolerate.
Also can you ever have a value that is not in fact f64 for a value typechecked pr
Through DuckDB, no. Its binder coerces to DOUBLE before pushdown. But Vortex itself never coerces filter expressions (coerce_expression exists but has no callers), and Binary's typecheck exempts extension-typed operands, so the rule assumes uncoerced input from any engine: cast what's numeric, decline the rest (never error). The new tests pin exactly that.
…minology sweep - Rename GeometryBounds -> GeometryAabb (id vortex.geo.bounds -> vortex.geo.aabb), BoundsPartial -> AabbPartial, aggregate_fn/bounds.rs -> aabb.rs: the statistic is specifically the axis-aligned bounding box, leaving room for other bbox kinds later - Rename GeoDistanceBoundsPrune -> GeoDistancePrune; the AABB is an implementation detail carried by the doc comment - Sweep docs, comments, and test names to axis-aligned bounding box (AABB) terminology - Document what zone_stat_default is for and note zone_stat_defaults' linear registry scan (intended once per column at writer open) - Present GeometryAabb as a plain aggregate whose zone-stat role is an application - Collapse AabbPartial::merge to map_or - Comment which other operators could prune in the future (==) or cannot (!=), and why the radius try_from fall-through is a sound decline rather than an error - Extract test_harness::geo_session() to replace repeated session+initialize in tests Signed-off-by: Nemo Yu <zyu379@wisc.edu>
joseph-isaacs
left a comment
joseph-isaacs
left a comment
There was a problem hiding this comment.
LG, there are three or so optional comments
…stic - Add GeoIntersectsPrune: skips chunks whose AABB is strictly separated from the filter constant's bounding box (touching boxes scan, since touching geometries intersect); NOT-intersects and unprovable shapes fall through to the scan - Restructure prune.rs into a directory module ready for future spatial predicates: mod.rs holds the soundness model and the shared vocabulary (geometry_and_constant, query_aabb, aabb_stat, min_dist_sq/max_dist_sq), one leaf module per predicate (distance.rs, intersects.rs), and tests in tests.rs with a shared zone-map harness - Fold in the #8646 review follow-ups that missed its merge: the aabb_stat ctor (part of the restructure) and tests pinning the radius-literal tolerance set - an integer radius casts and prunes, null and extension-typed radii decline to a scan; the harness now takes any literal scalar Signed-off-by: Nemo Yu <zyu379@wisc.edu>
…stic - Add GeoIntersectsPrune: skips chunks whose AABB is strictly separated from the filter constant's bounding box (touching boxes scan, since touching geometries intersect); NOT-intersects and unprovable shapes fall through to the scan - Restructure prune.rs into a directory module ready for future spatial predicates: mod.rs holds the shared proof vocabulary (geometry_and_constant, query_aabb, aabb_stat, min_dist_sq/max_dist_sq), each predicate is a leaf module owning its own test cases (distance.rs, intersects.rs), and shared zone-map fixtures live in test_harness.rs - Fold in the #8646 review follow-ups that missed its merge: the aabb_stat ctor (part of the restructure) and tests pinning the radius-literal tolerance set - an integer radius casts and prunes, null and extension-typed radii decline to a scan; the harness takes any literal scalar Signed-off-by: Nemo Yu <zyu379@wisc.edu>
…stics (#8775) ## Rationale for this change Follow-up to #8646, which added the `GeometryAabb` zone-map statistic and used it to prune `ST_Distance(geom, const) <op> r` filters. This PR adds the second spatial predicate on the same statistic: `ST_Intersects(geom, const)`. A chunk whose AABB is strictly separated from the constant's bounding box cannot contain an intersecting row, so the zone is skipped without any IO. This PR also folds in two review follow-ups from #8646 that were pushed minutes after it merged: tests pinning the radius-literal tolerance set, and the `aabb_stat` constructor requested in review. ## What changes are included in this PR? - **`GeoIntersectsPrune`** (`prune/intersects.rs`): The proof is `min_dist_sq(chunk_aabb, query_aabb) > 0`, strictly: boxes that merely touch must scan, since touching geometries intersect under OGC semantics. - **`prune/` restructure**. - **#8646 review follow-ups**: the radius conversion's contract is documented and pinned by tests. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
3 participants
Summary
Adds spatial chunk-pruning to Vortex. A new
GeometryBoundsaggregate stores a per-chunk minimum bounding box (MBR) as a zone-map statistic, and a stats-rewrite rule uses it to skip chunks that cannot satisfy aST_Distance(geom, const) <= rfilter.Limitation
<=/<are pruned.>/>=are soundly prunable via the symmetric farthest-corner bound but are intentionally omitted (rarely?)Testing
8 new vortex-geo tests. Point bbox across batches; Polygon bbox over all ring vertices, empty group → null, and registry self-declaration. only <=/< prune while >/>=/==/!= don't (parameterized), distance symmetry, non-distance comparisons ignored, and an end-to-end falsify.
Performance
SF=1
SF=3
SF=10
Takeaway: when the data is pre-sorted, the bounding box pruning can significantly reduce the intermediate results DuckDB read in, make Q1 and Q3 significantly faster