paleolimbot commented on code in PR #283:
URL: https://github.com/apache/sedona-db/pull/283#discussion_r2542737821
##########
rust/sedona-expr/src/spatial_filter.rs:
##########
@@ -60,6 +64,44 @@ pub enum SpatialFilter {
}
impl SpatialFilter {
+ /// Compute the maximum extent of a filter for a specific column index
+ ///
+ /// Some spatial file formats have the ability to push down a bounding box
+ /// into an index. This function allows deriving that bounding box based
+ /// on what DataFusion provides, which is a physical expression.
+ ///
+ /// Note that this always succeeds; however, for a non-spatial expression
or
+ /// a non-spatial expression that is unsupported, the full bounding box is
+ /// returned.
+ pub fn filter_bbox(&self, column_index: usize) -> BoundingBox {
+ match self {
+ SpatialFilter::Intersects(column, bounding_box)
+ | SpatialFilter::Covers(column, bounding_box) => {
+ if column.index() == column_index {
+ return bounding_box.clone();
+ }
+ }
+ SpatialFilter::And(lhs, rhs) => {
+ let lhs_box = lhs.filter_bbox(column_index);
+ let rhs_box = rhs.filter_bbox(column_index);
+ if let Ok(bounds) = lhs_box.intersection(&rhs_box) {
+ return bounds;
+ }
+ }
+ SpatialFilter::Or(lhs, rhs) => {
+ let mut bounds = lhs.filter_bbox(column_index);
+ bounds.update_box(&rhs.filter_bbox(column_index));
+ return bounds;
+ }
+ SpatialFilter::LiteralFalse => {
+ return BoundingBox::xy(Interval::empty(), Interval::empty())
+ }
+ SpatialFilter::HasZ(_) | SpatialFilter::Unknown => {}
+ }
+
+ BoundingBox::xy(Interval::full(), Interval::full())
+ }
Review Comment:
@Kontinuation can you sanity check this?
##########
rust/sedona-geometry/src/interval.rs:
##########
@@ -486,6 +506,53 @@ impl IntervalTrait for WraparoundInterval {
// So the interval itself is (excluded_hi, excluded_lo)
Self::new(excluded_hi, excluded_lo)
}
+
+ fn intersection(&self, other: &Self) -> Result<Self, SedonaGeometryError> {
+ match (self.is_wraparound(), other.is_wraparound()) {
+ // Neither is wraparound
+ (false, false) =>
Ok(self.inner.intersection(&other.inner)?.into()),
+ // One is wraparound
+ (true, false) => other.intersection(self),
+ (false, true) => {
+ let inner = self.inner;
+ let (left, right) = other.split();
+ match (inner.intersects_interval(&left),
inner.intersects_interval(&right)) {
+ // Intersects both the left and right intervals
+ (true, true) => {
+ Err(SedonaGeometryError::Invalid(format!("Can't
represent the intersection of {self:?} and {other:?} as a single
WraparoundInterval")))
+ },
+ // Intersects only the left interval
+ (true, false) => Ok(inner.intersection(&left)?.into()),
+ // Intersects only the right interval
+ (false, true) => Ok(inner.intersection(&right)?.into()),
+ (false, false) => Ok(WraparoundInterval::empty()),
+ }
+ }
+ // Both are wraparound
+ (true, true) => {
+ // Both wraparound intervals represent complements of excluded
regions
+ // Intersection of complements = complement of union of
excluded regions
+ // self excludes (hi, lo), other excludes (other.hi, other.lo)
+ // We need to find the union of these excluded regions
+ let excluded_self = Interval::new(self.inner.hi,
self.inner.lo);
+ let excluded_other = Interval::new(other.inner.hi,
other.inner.lo);
+
+ // We can't use the excluded union if the excluded region of
self and other
+ // are disjoint
+ if excluded_self.intersects_interval(&excluded_other) {
+ let excluded_union =
excluded_self.merge_interval(&excluded_other);
+
+ // The intersection is the complement of the union of
excluded regions
+ Ok(WraparoundInterval::new(
+ excluded_union.hi(),
+ excluded_union.lo(),
+ ))
+ } else {
+ Err(SedonaGeometryError::Invalid(format!("Can't represent
the intersection of {self:?} and {other:?} as a single WraparoundInterval")))
+ }
+ }
+ }
Review Comment:
This is also a bit of non-trivial math that could use a sanity check (the
tests below have some ASCII art to help visualize the various cases)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
