From e09f43d899e6980efd41dd8cbd551f9913b078fc Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 26 Aug 2024 17:45:39 +0200 Subject: [PATCH 1/2] Remove `parse_vec_expr` helper Behavior of `parse_vec_expr` and `parse_exprs` is almost similar -- both take a collection of expressions to parse. The only difference is that `parse_vec_expr` returns `Option::None` when collections is empty, but this difference in behavior does not correspond to difference in function names. Since the function is used once only, remove it instead of coming up with a fancy name. --- datafusion/proto/src/logical_plan/from_proto.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index b74237b5281b8..acda1298dd80e 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -586,7 +586,10 @@ pub fn parse_expr( parse_exprs(&pb.args, registry, codec)?, pb.distinct, parse_optional_expr(pb.filter.as_deref(), registry, codec)?.map(Box::new), - parse_vec_expr(&pb.order_by, registry, codec)?, + match pb.order_by.len() { + 0 => None, + _ => Some(parse_exprs(&pb.order_by, registry, codec)?), + }, None, ))) } @@ -676,16 +679,6 @@ pub fn from_proto_binary_op(op: &str) -> Result { } } -fn parse_vec_expr( - p: &[protobuf::LogicalExprNode], - registry: &dyn FunctionRegistry, - codec: &dyn LogicalExtensionCodec, -) -> Result>, Error> { - let res = parse_exprs(p, registry, codec)?; - // Convert empty vector to None. - Ok((!res.is_empty()).then_some(res)) -} - fn parse_optional_expr( p: Option<&protobuf::LogicalExprNode>, registry: &dyn FunctionRegistry, From 925dae8daf1d5c6262b713e3a6472ab42b475530 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 26 Aug 2024 18:44:25 +0200 Subject: [PATCH 2/2] empty