alamb commented on code in PR #6122:
URL: https://github.com/apache/arrow-datafusion/pull/6122#discussion_r1177688659
##########
datafusion/physical-expr/src/expressions/datetime.rs:
##########
@@ -37,45 +37,16 @@ pub struct DateTimeIntervalExpr {
lhs: Arc<dyn PhysicalExpr>,
op: Operator,
rhs: Arc<dyn PhysicalExpr>,
- // TODO: move type checking to the planning phase and not in the physical
expr
Review Comment:
🎉
##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -172,36 +171,36 @@ where
}
/// Creates a `Box<dyn Set>` for the given list of `IN` expressions and `batch`
-fn make_set(array: &dyn Array) -> Result<Box<dyn Set>> {
+fn make_set(array: &dyn Array) -> Result<Arc<dyn Set>> {
Review Comment:
This is a clever idea -- to avoid cloning the set over and over again 👍
##########
datafusion/physical-expr/src/expressions/mod.rs:
##########
@@ -78,7 +78,7 @@ pub use cast::{
cast, cast_column, cast_with_options, CastExpr,
DEFAULT_DATAFUSION_CAST_OPTIONS,
};
pub use column::{col, Column, UnKnownColumn};
-pub use datetime::DateTimeIntervalExpr;
+pub use datetime::{date_time_interval_expr, DateTimeIntervalExpr};
Review Comment:
👍
##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -586,28 +586,7 @@ pub fn reassign_predicate_columns(
column.name(),
index,
))));
- } else if let Some(in_list) = expr_any.downcast_ref::<InListExpr>() {
Review Comment:
🎉
##########
datafusion/physical-expr/src/expressions/datetime.rs:
##########
@@ -220,6 +190,36 @@ impl PartialEq<dyn Any> for DateTimeIntervalExpr {
}
}
+/// create a DateIntervalExpr
+pub fn date_time_interval_expr(
Review Comment:
👍 this also follows the model of the other creation functions in physical
expr.
--
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]
