alamb commented on code in PR #9343:
URL: https://github.com/apache/arrow-datafusion/pull/9343#discussion_r1506437221
##########
datafusion/proto/tests/cases/roundtrip_logical_plan.rs:
##########
@@ -578,7 +578,14 @@ async fn roundtrip_expr_api() -> Result<()> {
let expr_list = vec![
encode(col("a").cast_to(&DataType::Utf8, &schema)?, lit("hex")),
decode(lit("1234"), lit("hex")),
- array_to_string(array(vec![lit(1), lit(2), lit(3)]), lit(",")),
+ array_to_string(make_array(vec![lit(1), lit(2), lit(3)]), lit(",")),
Review Comment:
👍
##########
datafusion/optimizer/Cargo.toml:
##########
@@ -44,6 +44,7 @@ async-trait = { workspace = true }
chrono = { workspace = true }
datafusion-common = { workspace = true }
datafusion-expr = { workspace = true }
+datafusion-functions-array = { workspace = true }
Review Comment:
I am not sure about this new dependency -- it means that using the optimizer
will require bringing in the physical exprs, etc and users (like dask-sql) that
only want the planner code would bring in substantial unused code.
I realize the reason this is required is to preserve the semantics of the
existing rewrite pass in `datafusion/optimizer/src/analyzer/rewrite_expr.rs`. I
wonder if we can somehow avoid adding this new dependency 🤔
--
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]
