Repository navigation
Implement physical planner support for DATE +/- INTERVAL - #2357
Conversation
| let plan = ctx.create_logical_plan(sql).expect(&msg); | ||
| let plan = ctx | ||
| .create_logical_plan(sql) | ||
| .map_err(|e| format!("{:?} at {}", e, msg)) |
There was a problem hiding this comment.
This makes the tests show the actual error that happened so makes debugging much easier
andygrove
changed the title
| pub(crate) fn evaluate_to_scalar(&self, expr: &Expr) -> Result<ScalarValue> { | ||
| if let Expr::Literal(s) = expr { | ||
| return Ok(s); | ||
| return Ok(s.clone()); |
There was a problem hiding this comment.
The clone is unfortunate -- I wonder if there is some way to pass it back. Like for example a signature such as he following (aka return the Expr as the error if the evaluation doesn't work)?
Maybe with some debugging messages on error (via debug!) would be sufficent?
pub(crate) fn evaluate_to_scalar(&self, expr: Expr) -> std::result::Result<ScalarValue, Expr> {There was a problem hiding this comment.
@alamb I reverted this change and actually completely changed direction on this PR ... please see the updated PR description.
andygrove
changed the title
DateIntervalExpr physical expression
andygrove
changed the title
DateIntervalExpr physical expression
andygrove
changed the title
| execution_props, | ||
| )?; | ||
| binary(lhs, *op, rhs, input_schema) | ||
| match ( |
There was a problem hiding this comment.
I like this approach to use a specialized operator
I think over the long term, the ideal outcome would be to extend the arrow temporal kernels https://github.com/apache/arrow-rs/blob/master/arrow/src/compute/kernels/temporal.rs with the actual calculation logic.
However, incubating the kernels within datafusion seems like a great idea
Which issue does this PR close?
Closes #2236
Rationale for this change
Increasing our SQL support.
What changes are included in this PR?
DateIntervalExprphysical expression. Note that for now it only supports scalar inputs. This is enough to support SQL expressions such ascast('1999-4-01' as date) + INTERVAL '60 DAYS'which get simplified during optimization and replaced with a scalar value.Are there any user-facing changes?
No