thinkharderdev commented on code in PR #68:
URL: https://github.com/apache/arrow-ballista/pull/68#discussion_r898157589
##########
ballista/rust/core/src/serde/physical_plan/mod.rs:
##########
@@ -235,7 +235,11 @@ impl AsExecutionPlan for PhysicalPlanNode {
PhysicalPlanType::GlobalLimit(limit) => {
let input: Arc<dyn ExecutionPlan> =
into_physical_plan!(limit.input, registry, runtime,
extension_codec)?;
- Ok(Arc::new(GlobalLimitExec::new(input, limit.limit as usize)))
+ Ok(Arc::new(GlobalLimitExec::new(
Review Comment:
Seems like here we should just update the protobuf struct to hold both skip
and fetch. In our downstream I just used `0` to encode `None`. I _think_ that
should work unless having `fetch = Some(0)` is meaningful in some scenario
##########
ballista/rust/core/src/serde/physical_plan/mod.rs:
##########
@@ -235,7 +235,11 @@ impl AsExecutionPlan for PhysicalPlanNode {
PhysicalPlanType::GlobalLimit(limit) => {
let input: Arc<dyn ExecutionPlan> =
into_physical_plan!(limit.input, registry, runtime,
extension_codec)?;
- Ok(Arc::new(GlobalLimitExec::new(input, limit.limit as usize)))
+ Ok(Arc::new(GlobalLimitExec::new(
Review Comment:
See
https://github.com/coralogix/arrow-ballista/commit/68aecb8e7e16ce7346dac3192153e000d3f0dfe4
##########
ballista/rust/core/src/serde/physical_plan/mod.rs:
##########
@@ -799,11 +803,13 @@ impl AsExecutionPlan for PhysicalPlanNode {
} else if let Some(exec) = plan.downcast_ref::<AggregateExec>() {
let groups = exec
.group_expr()
+ .expr()
.iter()
.map(|expr| expr.0.to_owned().try_into())
.collect::<Result<Vec<_>, BallistaError>>()?;
let group_names = exec
.group_expr()
+ .expr()
.iter()
.map(|expr| expr.1.to_owned())
.collect();
Review Comment:
Here we need to serialize/deserialize the `null_expr` and `groups` as well.
This would change the structure of `GROUPING SET` queries. I had to do the same
changes on our downstream fork. This seems to work:
https://github.com/coralogix/arrow-ballista/commit/68aecb8e7e16ce7346dac3192153e000d3f0dfe4
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
