alamb commented on code in PR #3444:
URL: https://github.com/apache/arrow-datafusion/pull/3444#discussion_r968404125
##########
datafusion/core/tests/sql/aggregates.rs:
##########
@@ -1834,11 +1834,11 @@ async fn aggregate_avg_add() -> Result<()> {
assert_eq!(results.len(), 1);
let expected = vec![
-
"+--------------+-------------------------+-------------------------+-------------------------+",
- "| AVG(test.c1) | AVG(test.c1) + Int64(1) | AVG(test.c1) + Int64(2) |
Int64(1) + AVG(test.c1) |",
-
"+--------------+-------------------------+-------------------------+-------------------------+",
- "| 1.5 | 2.5 | 3.5 |
2.5 |",
-
"+--------------+-------------------------+-------------------------+-------------------------+",
+
"+--------------+---------------------------+---------------------------+---------------------------+",
Review Comment:
I really like the idea of adding an alias once (maybe as the initial
optimizer pass?)
I am not sure how valuable adding the types in the column names is in
general, to be honest. I wouldn't mind if rather than `Int(1)` this was simply
rendered `1`
##########
datafusion/core/tests/sql/explain_analyze.rs:
##########
@@ -653,7 +653,7 @@ order by
let expected = "\
Sort: #revenue DESC NULLS FIRST\
\n Projection: #customer.c_custkey, #customer.c_name,
#SUM(lineitem.l_extendedprice * Int64(1) - lineitem.l_discount) AS revenue,
#customer.c_acctbal, #nation.n_name, #customer.c_address, #customer.c_phone,
#customer.c_comment\
- \n Aggregate: groupBy=[[#customer.c_custkey, #customer.c_name,
#customer.c_acctbal, #customer.c_phone, #nation.n_name, #customer.c_address,
#customer.c_comment]], aggr=[[SUM(#lineitem.l_extendedprice * CAST(Int64(1) AS
Float64) - #lineitem.l_discount)]]\
+ \n Aggregate: groupBy=[[#customer.c_custkey, #customer.c_name,
#customer.c_acctbal, #customer.c_phone, #nation.n_name, #customer.c_address,
#customer.c_comment]], aggr=[[SUM(#lineitem.l_extendedprice * Float64(1) -
#lineitem.l_discount)]]\
Review Comment:
very nice
##########
datafusion/optimizer/src/type_coercion.rs:
##########
@@ -64,7 +66,15 @@ impl OptimizerRule for TypeCoercion {
_ => DFSchemaRef::new(DFSchema::empty()),
};
- let mut expr_rewrite = TypeCoercionRewriter { schema };
+ let mut execution_props = ExecutionProps::new();
+ execution_props.query_execution_start_time =
+ optimizer_config.query_execution_start_time;
+ let const_evaluator = ConstEvaluator::try_new(&execution_props)?;
Review Comment:
I think the const evaluator is somewhat expensive to run (but not
construct); We should keep an eye on the performance of query planning, but I
think this is reasonable for now
--
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]
