Copilot commented on code in PR #39:
URL: https://github.com/apache/sedona-db/pull/39#discussion_r2333710168
##########
rust/sedona-geoparquet/src/format.rs:
##########
@@ -352,7 +352,7 @@ impl GeoParquetFileSource {
if let Some(parquet_source) =
inner.as_any().downcast_ref::<ParquetSource>() {
let mut parquet_source = parquet_source.clone();
- // Extract the precicate from the existing source if it exists so
we can keep a copy of it
+ // Extract the predicate from the existing source if it exists so
we can keep a copy of it
Review Comment:
Fixed typo from 'precicate' to 'predicate' in comment.
##########
rust/sedona-expr/src/spatial_filter.rs:
##########
@@ -187,6 +167,144 @@ impl SpatialFilter {
Ok(Self::Unknown)
}
}
+
+ fn try_from_range_predicate(expr: &Arc<dyn PhysicalExpr>) ->
Result<Option<Self>> {
+ let Some(scalar_fun) =
expr.as_any().downcast_ref::<ScalarFunctionExpr>() else {
+ return Ok(None);
+ };
+
+ let raw_args = scalar_fun.args();
+ let args = parse_args(raw_args);
+ let fun_name = scalar_fun.fun().name();
+ match fun_name {
+ "st_intersects" | "st_equals" | "st_touches" => {
+ if args.len() != 2 {
+ return sedona_internal_err!("unexpected argument count in
filter evaluation");
+ }
+
+ match (&args[0], &args[1]) {
+ (ArgRef::Col(column), ArgRef::Lit(literal))
+ | (ArgRef::Lit(literal), ArgRef::Col(column)) => {
+ match literal_bounds(literal) {
+ Ok(literal_bounds) => {
+ Ok(Some(Self::Intersects(column.clone(),
literal_bounds)))
+ }
+ Err(e) =>
Err(DataFusionError::External(Box::new(e))),
+ }
+ }
+ // Not between a literal and a column
+ _ => Ok(Some(Self::Unknown)),
+ }
+ }
+ "st_within" | "st_covered_by" | "st_coveredby" => {
+ if args.len() != 2 {
+ return sedona_internal_err!("unexpected argument count in
filter evaluation");
+ }
+
+ match (&args[0], &args[1]) {
+ (ArgRef::Col(column), ArgRef::Lit(literal)) => {
+ // column within/covered_by literal -> CoveredBy filter
+ match literal_bounds(literal) {
+ Ok(literal_bounds) => {
+ Ok(Some(Self::CoveredBy(column.clone(),
literal_bounds)))
+ }
+ Err(e) =>
Err(DataFusionError::External(Box::new(e))),
+ }
+ }
+ (ArgRef::Lit(literal), ArgRef::Col(column)) => {
+ // literal within/covered_by column -> Intersects
filter
+ match literal_bounds(literal) {
+ Ok(literal_bounds) => {
+ Ok(Some(Self::Intersects(column.clone(),
literal_bounds)))
+ }
+ Err(e) =>
Err(DataFusionError::External(Box::new(e))),
+ }
+ }
+ // Not between a literal and a column
+ _ => Ok(Some(Self::Unknown)),
+ }
+ }
+ "st_contains" | "st_covers" => {
+ if args.len() != 2 {
+ return sedona_internal_err!("unexpected argument count in
filter evaluation");
+ }
+
+ match (&args[0], &args[1]) {
+ (ArgRef::Col(column), ArgRef::Lit(literal)) => {
+ // column contains/covers literal -> Intersects filter
+ // (column must potentially intersect literal to
contain it)
+ match literal_bounds(literal) {
+ Ok(literal_bounds) => {
+ Ok(Some(Self::Intersects(column.clone(),
literal_bounds)))
+ }
+ Err(e) =>
Err(DataFusionError::External(Box::new(e))),
+ }
+ }
+ (ArgRef::Lit(literal), ArgRef::Col(column)) => {
+ // literal contains/covers column -> CoveredBy filter
+ // (equivalent to st_within(column, literal))
+ match literal_bounds(literal) {
+ Ok(literal_bounds) => {
+ Ok(Some(Self::CoveredBy(column.clone(),
literal_bounds)))
+ }
+ Err(e) =>
Err(DataFusionError::External(Box::new(e))),
+ }
+ }
+ // Not between a literal and a column
+ _ => Ok(Some(Self::Unknown)),
+ }
+ }
+ "st_hasz" => {
+ if args.len() != 1 {
+ return sedona_internal_err!("unexpected argument count in
filter evaluation");
+ }
+
+ match &args[0] {
+ ArgRef::Col(column) =>
Ok(Some(Self::HasZ(column.clone()))),
+ _ => Ok(Some(Self::Unknown)),
+ }
+ }
+ _ => Ok(None),
+ }
+ }
+
+ fn try_from_distance_predicate(expr: &Arc<dyn PhysicalExpr>) ->
Result<Option<Self>> {
+ let Some(ParsedDistancePredicate {
+ arg0,
+ arg1,
+ arg_distance,
+ }) = parse_distance_predicate(expr)
+ else {
+ return Ok(None);
+ };
+
+ let raw_args = [arg0, arg1, arg_distance];
+ let args = parse_args(&raw_args);
+
+ match (&args[0], &args[1], &args[2]) {
+ (ArgRef::Col(column), ArgRef::Lit(literal), ArgRef::Lit(distance))
+ | (ArgRef::Lit(literal), ArgRef::Col(column),
ArgRef::Lit(distance)) => {
+ match (
+ literal_bounds(literal),
+ distance.value().cast_to(&DataType::Float64)?,
Review Comment:
[nitpick] The cast_to method uses None for the second parameter, but this
should be explicitly provided for clarity. Consider using
`cast_to(&DataType::Float64, None)?` to make the intent clearer.
```suggestion
distance.value().cast_to(&DataType::Float64, None)?,
```
##########
rust/sedona-spatial-join/src/operand_evaluator.rs:
##########
@@ -240,6 +241,7 @@ impl DistanceOperandEvaluator {
// Expand the vec by distance
let distance_columnar_value = self.inner.distance.evaluate(batch)?;
Review Comment:
[nitpick] The cast_to method uses None for the second parameter, but this
should be explicitly provided for clarity. Consider making the intent of the
None parameter more explicit through documentation or a named constant.
```suggestion
let distance_columnar_value = self.inner.distance.evaluate(batch)?;
// No timezone conversion needed for distance; pass None explicitly.
```
--
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]
