dharanad commented on code in PR #10964:
URL: https://github.com/apache/datafusion/pull/10964#discussion_r1649703615
##########
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -281,17 +283,6 @@ fn roundtrip_window() -> Result<()> {
Arc::new(window_frame),
));
- let plain_aggr_window_expr = Arc::new(PlainAggregateWindowExpr::new(
Review Comment:
My bad, i might have remove it while refactoring it. But the test case is
failing with below error.
```
thread 'cases::roundtrip_physical_plan::roundtrip_window' panicked at
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:113:14:
to proto: NotImplemented("Aggregate function not supported:
AggregateFunctionExpr { fun: AggregateUDF { inner: Avg { signature: Signature {
type_signature: UserDefined, volatility: Immutable }, aliases: [\"mean\"] } },
args: [Column { name: \"b\", index: 1 }], logical_args: [], data_type: Float64,
name: \"AVG(b)\", schema: Schema { fields: [Field { name: \"a\", data_type:
Int64, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} },
Field { name: \"b\", data_type: Int64, nullable: false, dict_id: 0,
dict_is_ordered: false, metadata: {} }], metadata: {} }, sort_exprs: [],
ordering_req: [], ignore_nulls: false, ordering_fields: [], is_distinct: false,
input_type: Int64 }")
```
This is occuring because we have removed
```rust
fn aggr_expr_to_aggr_fn(expr: &dyn AggregateExpr) -> Result<AggrFn> {
...
} else if aggr_expr.downcast_ref::<Avg>().is_some() {
protobuf::AggregateFunction::Avg
}
...
```
I thinking we can remove this test case from the physical plan and include
it in the logical plan instead?
I'm still learning about query engines, so please forgive me if my
understanding is incomplete
##########
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -281,17 +283,6 @@ fn roundtrip_window() -> Result<()> {
Arc::new(window_frame),
));
- let plain_aggr_window_expr = Arc::new(PlainAggregateWindowExpr::new(
Review Comment:
@jayzhan211 My bad, i might have removed it while refactoring it. But the
test case is failing with below error.
```
thread 'cases::roundtrip_physical_plan::roundtrip_window' panicked at
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:113:14:
to proto: NotImplemented("Aggregate function not supported:
AggregateFunctionExpr { fun: AggregateUDF { inner: Avg { signature: Signature {
type_signature: UserDefined, volatility: Immutable }, aliases: [\"mean\"] } },
args: [Column { name: \"b\", index: 1 }], logical_args: [], data_type: Float64,
name: \"AVG(b)\", schema: Schema { fields: [Field { name: \"a\", data_type:
Int64, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} },
Field { name: \"b\", data_type: Int64, nullable: false, dict_id: 0,
dict_is_ordered: false, metadata: {} }], metadata: {} }, sort_exprs: [],
ordering_req: [], ignore_nulls: false, ordering_fields: [], is_distinct: false,
input_type: Int64 }")
```
This is occuring because we have removed
```rust
fn aggr_expr_to_aggr_fn(expr: &dyn AggregateExpr) -> Result<AggrFn> {
...
} else if aggr_expr.downcast_ref::<Avg>().is_some() {
protobuf::AggregateFunction::Avg
}
...
```
I thinking we can remove this test case from the physical plan and include
it in the logical plan instead?
I'm still learning about query engines, so please forgive me if my
understanding is incomplete
##########
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -281,17 +283,6 @@ fn roundtrip_window() -> Result<()> {
Arc::new(window_frame),
));
- let plain_aggr_window_expr = Arc::new(PlainAggregateWindowExpr::new(
Review Comment:
I have added a test case in `fn roundtrip_window` in
`datafusion/proto/tests/cases/roundtrip_logical_plan.rs`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
