Skip to content

Commit a6d4798

Browse files
authored
Fixes 3 bugs during serialization and deserialization of physical plans (#16858)
1 parent 3b4eda5 commit a6d4798

File tree

12 files changed

+163
-7
lines changed

12 files changed

+163
-7
lines changed

Cargo.lock

Lines changed: 23 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/core/src/physical_planner.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,6 +1358,9 @@ impl DefaultPhysicalPlanner {
13581358
physical_name(expr),
13591359
))?])),
13601360
}
1361+
} else if group_expr.is_empty() {
1362+
// No GROUP BY clause - create empty PhysicalGroupBy
1363+
Ok(PhysicalGroupBy::new(vec![], vec![], vec![]))
13611364
} else {
13621365
Ok(PhysicalGroupBy::new_single(
13631366
group_expr

datafusion/ffi/src/udaf/accumulator_args.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ impl TryFrom<AccumulatorArgs<'_>> for FFI_AccumulatorArgs {
7575
ignore_nulls: args.ignore_nulls,
7676
fun_definition: None,
7777
aggregate_function: None,
78+
human_display: args.name.to_string(),
7879
};
7980
let physical_expr_def = physical_expr_def.encode_to_vec().into();
8081

datafusion/physical-plan/src/aggregates/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,17 @@ impl PhysicalGroupBy {
332332
)
333333
.collect();
334334
let num_exprs = expr.len();
335+
let groups = if self.expr.is_empty() {
336+
// No GROUP BY expressions - should have no groups
337+
vec![]
338+
} else {
339+
// Has GROUP BY expressions - create a single group
340+
vec![vec![false; num_exprs]]
341+
};
335342
Self {
336343
expr,
337344
null_expr: vec![],
338-
groups: vec![vec![false; num_exprs]],
345+
groups,
339346
}
340347
}
341348
}

datafusion/proto/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,5 @@ datafusion-functions = { workspace = true, default-features = true }
6060
datafusion-functions-aggregate = { workspace = true }
6161
datafusion-functions-window-common = { workspace = true }
6262
doc-comment = { workspace = true }
63+
pretty_assertions = "1.4"
6364
tokio = { workspace = true, features = ["rt-multi-thread"] }

datafusion/proto/proto/datafusion.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,7 @@ message PhysicalScalarUdfNode {
859859
optional bytes fun_definition = 3;
860860
datafusion_common.ArrowType return_type = 4;
861861
bool nullable = 5;
862+
string return_field_name = 6;
862863
}
863864

864865
message PhysicalAggregateExprNode {
@@ -870,6 +871,7 @@ message PhysicalAggregateExprNode {
870871
bool distinct = 3;
871872
bool ignore_nulls = 6;
872873
optional bytes fun_definition = 7;
874+
string human_display = 8;
873875
}
874876

875877
message PhysicalWindowExprNode {

datafusion/proto/src/generated/pbjson.rs

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/proto/src/generated/prost.rs

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/proto/src/physical_plan/from_proto.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,12 @@ pub fn parse_physical_expr(
368368
e.name.as_str(),
369369
scalar_fun_def,
370370
args,
371-
Field::new("f", convert_required!(e.return_type)?, true).into(),
371+
Field::new(
372+
&e.return_field_name,
373+
convert_required!(e.return_type)?,
374+
true,
375+
)
376+
.into(),
372377
)
373378
.with_nullable(e.nullable),
374379
)

datafusion/proto/src/physical_plan/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,7 @@ impl protobuf::PhysicalPlanNode {
11081108
AggregateExprBuilder::new(agg_udf, input_phy_expr)
11091109
.schema(Arc::clone(&physical_schema))
11101110
.alias(name)
1111+
.human_display(agg_node.human_display.clone())
11111112
.with_ignore_nulls(agg_node.ignore_nulls)
11121113
.with_distinct(agg_node.distinct)
11131114
.order_by(order_bys)

0 commit comments

Comments
 (0)