You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/23 15:45:01 UTC
[GitHub] [arrow] andygrove opened a new pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
andygrove opened a new pull request #8746:
URL: https://github.com/apache/arrow/pull/8746
This PR builds on https://github.com/apache/arrow/pull/8740 and introduces functions and builders for creating CASE WHEN statements as part of the logical plan via the DataFrame API.
Here is an example from TPC-H query 12:
```rust
sum(case_when()
.when(or(
col("o_orderpriority").eq(lit("1-URGENT")),
col("o_orderpriority").eq(lit("2-HIGH")),
))
.then(lit(1))
.or_else(lit(0)))
.alias("high_line_count"),
sum(case_when()
.when(and(
col("o_orderpriority").not_eq(lit("1-URGENT")),
col("o_orderpriority").not_eq(lit("2-HIGH")),
))
.then(lit(1))
.or_else(lit(0)))
.alias("high_line_count"),
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#issuecomment-732243918
https://issues.apache.org/jira/browse/ARROW-10688
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r528909049
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -216,26 +238,51 @@ pub fn from_plan(
/// Returns all direct children `Expression`s of `expr`.
/// E.g. if the expression is "(a + 1) + 1", it returns ["a + 1", "1"] (as Expr objects)
-pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<&Expr>> {
+pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
match expr {
- Expr::BinaryExpr { left, right, .. } => Ok(vec![left, right]),
- Expr::IsNull(e) => Ok(vec![e]),
- Expr::IsNotNull(e) => Ok(vec![e]),
- Expr::ScalarFunction { args, .. } => Ok(args.iter().collect()),
- Expr::ScalarUDF { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateFunction { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateUDF { args, .. } => Ok(args.iter().collect()),
- Expr::Cast { expr, .. } => Ok(vec![expr]),
+ Expr::BinaryExpr { left, right, .. } => {
+ Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
+ }
+ Expr::IsNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::IsNotNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::ScalarFunction { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::ScalarUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::AggregateFunction { args, .. } => {
+ Ok(args.iter().map(|e| e.clone()).collect())
+ }
+ Expr::AggregateUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::Case {
+ expr,
+ when_then_expr,
+ else_expr,
+ ..
+ } => {
+ let mut expr_list: Vec<Expr> = vec![];
Review comment:
This felt a bit hacky but our expression rewrite mechanism doesn't really work well for this operator which has multiple types of expressions. We probably need to come up with something better long term.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r532032034
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -821,3 +851,26 @@ fn create_name(e: &Expr, input_schema: &Schema) -> Result<String> {
pub fn exprlist_to_fields(expr: &[Expr], input_schema: &Schema) -> Result<Vec<Field>> {
expr.iter().map(|e| e.to_field(input_schema)).collect()
}
+
+#[cfg(test)]
+mod tests {
+ use super::super::{col, lit, when};
+ use super::*;
+
+ #[test]
+ fn case_when_same_literal_then_types() -> Result<()> {
+ let _ = when(col("state").eq(lit("CO")), lit(303))
+ .when(col("state").eq(lit("NY")), lit(212))
+ .end()?;
+ Ok(())
+ }
+
+ #[test]
+ fn case_when_different_literal_then_types() -> Result<()> {
+ let maybe_expr = when(col("state").eq(lit("CO")), lit(303))
+ .when(col("state").eq(lit("NY")), lit("212"))
Review comment:
this is a classic error (string with a number) 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529223608
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -342,6 +386,82 @@ impl Expr {
}
}
+pub struct CaseWhenBuilder {
+ expr: Option<Box<Expr>>,
+ when_expr: Vec<Expr>,
+ then_expr: Vec<Expr>,
+}
+
+pub struct CaseThenBuilder {
+ expr: Option<Box<Expr>>,
+ when_expr: Vec<Expr>,
+ then_expr: Vec<Expr>,
+}
+
+impl CaseWhenBuilder {
+ pub fn when(&mut self, expr: Expr) -> CaseThenBuilder {
+ self.when_expr.push(expr);
+ CaseThenBuilder {
+ expr: self.expr.clone(),
+ when_expr: self.when_expr.clone(),
+ then_expr: self.then_expr.clone(),
+ }
+ }
+ pub fn or_else(&mut self, else_expr: Expr) -> Expr {
+ Expr::Case {
+ expr: self.expr.clone(),
+ when_then_expr: self
+ .when_expr
+ .iter()
+ .zip(self.then_expr.iter())
+ .map(|(w, t)| (Box::new(w.clone()), Box::new(t.clone())))
+ .collect(),
+ else_expr: Some(Box::new(else_expr)),
+ }
+ }
+ pub fn end(&mut self) -> Expr {
+ Expr::Case {
+ expr: self.expr.clone(),
+ when_then_expr: self
+ .when_expr
+ .iter()
+ .zip(self.then_expr.iter())
+ .map(|(w, t)| (Box::new(w.clone()), Box::new(t.clone())))
+ .collect(),
+ else_expr: None,
+ }
+ }
+}
Review comment:
I was aiming for a fluent-style API but this could work too. I will take a look at some other DataFrame APIs tomorrow (pandas, spark, etc) and see how they do it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529051136
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -216,26 +238,51 @@ pub fn from_plan(
/// Returns all direct children `Expression`s of `expr`.
/// E.g. if the expression is "(a + 1) + 1", it returns ["a + 1", "1"] (as Expr objects)
-pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<&Expr>> {
+pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
match expr {
- Expr::BinaryExpr { left, right, .. } => Ok(vec![left, right]),
- Expr::IsNull(e) => Ok(vec![e]),
- Expr::IsNotNull(e) => Ok(vec![e]),
- Expr::ScalarFunction { args, .. } => Ok(args.iter().collect()),
- Expr::ScalarUDF { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateFunction { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateUDF { args, .. } => Ok(args.iter().collect()),
- Expr::Cast { expr, .. } => Ok(vec![expr]),
+ Expr::BinaryExpr { left, right, .. } => {
+ Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
+ }
+ Expr::IsNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::IsNotNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::ScalarFunction { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::ScalarUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::AggregateFunction { args, .. } => {
+ Ok(args.iter().map(|e| e.clone()).collect())
+ }
+ Expr::AggregateUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::Case {
+ expr,
+ when_then_expr,
+ else_expr,
+ ..
+ } => {
+ let mut expr_list: Vec<Expr> = vec![];
Review comment:
The pattern I have seen elsewhere (aka that Postgres has) is two types of expression visitor -- one that just visits each node in an expression tree, and one that walks over the tree and returns a (potentially) rewritten node
I have an expr visitor here in IOx: https://github.com/influxdata/influxdb_iox/blob/659da9264ab39e9d0b2ea3a15c082f8c25fe0a68/storage/src/util.rs#L8
If you think it is a useful thing @andygrove -- I can take a shot at porting to Arrow
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529223287
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -24,7 +24,11 @@ use arrow::datatypes::{Schema, SchemaRef};
use super::optimizer::OptimizerRule;
use crate::error::{DataFusionError, Result};
use crate::logical_plan::{Expr, LogicalPlan, PlanType, StringifiedPlan};
-use crate::prelude::col;
+use crate::prelude::{col, lit};
+use crate::scalar::ScalarValue;
+
+const CASE_EXPR_MARKER: &str = "__DATAFUSION_CASE_EXPR__";
Review comment:
I couldn't think of another way to do the expression rewriting with our current design. This expression is quite different from the ones implemented so far.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529756391
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -216,26 +238,51 @@ pub fn from_plan(
/// Returns all direct children `Expression`s of `expr`.
/// E.g. if the expression is "(a + 1) + 1", it returns ["a + 1", "1"] (as Expr objects)
-pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<&Expr>> {
+pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
match expr {
- Expr::BinaryExpr { left, right, .. } => Ok(vec![left, right]),
- Expr::IsNull(e) => Ok(vec![e]),
- Expr::IsNotNull(e) => Ok(vec![e]),
- Expr::ScalarFunction { args, .. } => Ok(args.iter().collect()),
- Expr::ScalarUDF { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateFunction { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateUDF { args, .. } => Ok(args.iter().collect()),
- Expr::Cast { expr, .. } => Ok(vec![expr]),
+ Expr::BinaryExpr { left, right, .. } => {
+ Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
+ }
+ Expr::IsNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::IsNotNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::ScalarFunction { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::ScalarUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::AggregateFunction { args, .. } => {
+ Ok(args.iter().map(|e| e.clone()).collect())
+ }
+ Expr::AggregateUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::Case {
+ expr,
+ when_then_expr,
+ else_expr,
+ ..
+ } => {
+ let mut expr_list: Vec<Expr> = vec![];
Review comment:
I filed https://issues.apache.org/jira/browse/ARROW-10716 for us to follow up on this discussion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r528910229
##########
File path: rust/datafusion/src/optimizer/filter_push_down.rs
##########
@@ -93,7 +93,9 @@ fn issue_filters(
let predicate = predicates
.iter()
.skip(1)
- .fold(predicates[0].clone(), |acc, predicate| and(&acc, predicate));
+ .fold(predicates[0].clone(), |acc, predicate| {
+ and(acc, (*predicate).clone())
Review comment:
This is just cloning rather than using a reference when creating the `and` expression. `predicate` is `&&Expr` so I dereference with `*` to get `&Expr` and then clone (perhaps could be `to_owned` instead).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529223705
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -141,6 +166,7 @@ impl Expr {
Expr::Column(name) => Ok(schema.field_with_name(name)?.data_type().clone()),
Expr::ScalarVariable(_) => Ok(DataType::Utf8),
Expr::Literal(l) => Ok(l.get_datatype()),
+ Expr::Case { when_then_expr, .. } => when_then_expr[0].1.get_type(schema),
Review comment:
I agree. I can look at adding this tomorrow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#issuecomment-733076788
Thanks for the review @alamb. I have pushed some changes to address your feedback.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r528896150
##########
File path: rust/datafusion/src/optimizer/filter_push_down.rs
##########
@@ -93,7 +93,9 @@ fn issue_filters(
let predicate = predicates
.iter()
.skip(1)
- .fold(predicates[0].clone(), |acc, predicate| and(&acc, predicate));
+ .fold(predicates[0].clone(), |acc, predicate| {
+ and(acc, (*predicate).clone())
Review comment:
do you think it is worth adding a test to this? Looks non-trivial :/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] Dandandan commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r528945232
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -216,26 +238,51 @@ pub fn from_plan(
/// Returns all direct children `Expression`s of `expr`.
/// E.g. if the expression is "(a + 1) + 1", it returns ["a + 1", "1"] (as Expr objects)
-pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<&Expr>> {
+pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
match expr {
- Expr::BinaryExpr { left, right, .. } => Ok(vec![left, right]),
- Expr::IsNull(e) => Ok(vec![e]),
- Expr::IsNotNull(e) => Ok(vec![e]),
- Expr::ScalarFunction { args, .. } => Ok(args.iter().collect()),
- Expr::ScalarUDF { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateFunction { args, .. } => Ok(args.iter().collect()),
- Expr::AggregateUDF { args, .. } => Ok(args.iter().collect()),
- Expr::Cast { expr, .. } => Ok(vec![expr]),
+ Expr::BinaryExpr { left, right, .. } => {
+ Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
+ }
+ Expr::IsNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::IsNotNull(e) => Ok(vec![e.as_ref().to_owned()]),
+ Expr::ScalarFunction { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::ScalarUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::AggregateFunction { args, .. } => {
+ Ok(args.iter().map(|e| e.clone()).collect())
+ }
+ Expr::AggregateUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+ Expr::Case {
+ expr,
+ when_then_expr,
+ else_expr,
+ ..
+ } => {
+ let mut expr_list: Vec<Expr> = vec![];
Review comment:
FYI, I started/designed for the Polars library something a bit inspired by the Spark catalyst optimizer ( http://people.csail.mit.edu/matei/papers/2015/sigmod_spark_sql.pdf ), but a bit evolved in some ways to be useful in Rust. Specifically, it uses an arena to mutate the tree efficiently / without cloning.
Code lives here currently: https://github.com/ritchie46/polars/blob/master/polars/src/lazy/logical_plan/optimizer/simplify_expr.rs#L777
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529690058
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -141,6 +166,7 @@ impl Expr {
Expr::Column(name) => Ok(schema.field_with_name(name)?.data_type().clone()),
Expr::ScalarVariable(_) => Ok(DataType::Utf8),
Expr::Literal(l) => Ok(l.get_datatype()),
+ Expr::Case { when_then_expr, .. } => when_then_expr[0].1.get_type(schema),
Review comment:
I've added a check for the case where all "then" values are literals, to make sure they have the same type.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r529052558
##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -24,7 +24,11 @@ use arrow::datatypes::{Schema, SchemaRef};
use super::optimizer::OptimizerRule;
use crate::error::{DataFusionError, Result};
use crate::logical_plan::{Expr, LogicalPlan, PlanType, StringifiedPlan};
-use crate::prelude::col;
+use crate::prelude::{col, lit};
+use crate::scalar::ScalarValue;
+
+const CASE_EXPR_MARKER: &str = "__DATAFUSION_CASE_EXPR__";
Review comment:
🤔
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -141,6 +166,7 @@ impl Expr {
Expr::Column(name) => Ok(schema.field_with_name(name)?.data_type().clone()),
Expr::ScalarVariable(_) => Ok(DataType::Utf8),
Expr::Literal(l) => Ok(l.get_datatype()),
+ Expr::Case { when_then_expr, .. } => when_then_expr[0].1.get_type(schema),
Review comment:
I feel like there should be a check somewhere that all the `then` and `else` clauses result in the same type -- however, I suppose we would just get an arrow error at runtime, which while not ideal, could easily be fixed subsequently.
##########
File path: rust/datafusion/src/logical_plan/expr.rs
##########
@@ -342,6 +386,82 @@ impl Expr {
}
}
+pub struct CaseWhenBuilder {
+ expr: Option<Box<Expr>>,
+ when_expr: Vec<Expr>,
+ then_expr: Vec<Expr>,
+}
+
+pub struct CaseThenBuilder {
+ expr: Option<Box<Expr>>,
+ when_expr: Vec<Expr>,
+ then_expr: Vec<Expr>,
+}
+
+impl CaseWhenBuilder {
+ pub fn when(&mut self, expr: Expr) -> CaseThenBuilder {
+ self.when_expr.push(expr);
+ CaseThenBuilder {
+ expr: self.expr.clone(),
+ when_expr: self.when_expr.clone(),
+ then_expr: self.then_expr.clone(),
+ }
+ }
+ pub fn or_else(&mut self, else_expr: Expr) -> Expr {
+ Expr::Case {
+ expr: self.expr.clone(),
+ when_then_expr: self
+ .when_expr
+ .iter()
+ .zip(self.then_expr.iter())
+ .map(|(w, t)| (Box::new(w.clone()), Box::new(t.clone())))
+ .collect(),
+ else_expr: Some(Box::new(else_expr)),
+ }
+ }
+ pub fn end(&mut self) -> Expr {
+ Expr::Case {
+ expr: self.expr.clone(),
+ when_then_expr: self
+ .when_expr
+ .iter()
+ .zip(self.then_expr.iter())
+ .map(|(w, t)| (Box::new(w.clone()), Box::new(t.clone())))
+ .collect(),
+ else_expr: None,
+ }
+ }
+}
Review comment:
is there a reason for a second `impl` for `CaseThenBuilder`?
Also given that `when / then` always need to come in pairs, maybe
```
pub fn when(&mut self, expr: Expr) -> CaseThenBuilder {
```
Could be translated into
```
pub fn when_then(&mut self, when_expr: Expr, then_expr) -> CaseThenBuilder {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove closed pull request #8746:
URL: https://github.com/apache/arrow/pull/8746
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#issuecomment-732320989
@jorgecarleitao @alamb Please take a look when you can. This PR includes a bug fix in projection push-down as well, where columns where being thrown away due to re-creating the HashSet in some operators.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org