You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ag...@apache.org on 2022/08/24 23:12:09 UTC
[arrow-datafusion] branch master updated: MINOR: Stop ignoring `AggregateFunction::distinct` in protobuf serde code (#3250)
This is an automated email from the ASF dual-hosted git repository.
agrove pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new 7e407df8e MINOR: Stop ignoring `AggregateFunction::distinct` in protobuf serde code (#3250)
7e407df8e is described below
commit 7e407df8ec62345f783229eb9541e147813a501d
Author: Andy Grove <an...@gmail.com>
AuthorDate: Wed Aug 24 17:12:03 2022 -0600
MINOR: Stop ignoring `AggregateFunction::distinct` in protobuf serde code (#3250)
---
.../optimizer/src/single_distinct_to_groupby.rs | 2 +-
datafusion/proto/proto/datafusion.proto | 1 +
datafusion/proto/src/from_proto.rs | 2 +-
datafusion/proto/src/lib.rs | 22 ++++++++++++++++++++++
datafusion/proto/src/to_proto.rs | 5 ++++-
5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/datafusion/optimizer/src/single_distinct_to_groupby.rs b/datafusion/optimizer/src/single_distinct_to_groupby.rs
index 1769314eb..a4d6619f2 100644
--- a/datafusion/optimizer/src/single_distinct_to_groupby.rs
+++ b/datafusion/optimizer/src/single_distinct_to_groupby.rs
@@ -83,7 +83,7 @@ fn optimize(plan: &LogicalPlan) -> Result<LogicalPlan> {
Expr::AggregateFunction {
fun: fun.clone(),
args: vec![col(SINGLE_DISTINCT_ALIAS)],
- distinct: false,
+ distinct: false, // intentional to remove distict here
}
}
_ => agg_expr.clone(),
diff --git a/datafusion/proto/proto/datafusion.proto b/datafusion/proto/proto/datafusion.proto
index 7b08e4f40..0b4a43e83 100644
--- a/datafusion/proto/proto/datafusion.proto
+++ b/datafusion/proto/proto/datafusion.proto
@@ -474,6 +474,7 @@ enum AggregateFunction {
message AggregateExprNode {
AggregateFunction aggr_function = 1;
repeated LogicalExprNode expr = 2;
+ bool distinct = 3;
}
message AggregateUDFExprNode {
diff --git a/datafusion/proto/src/from_proto.rs b/datafusion/proto/src/from_proto.rs
index 63d9fe2b7..12f94ce36 100644
--- a/datafusion/proto/src/from_proto.rs
+++ b/datafusion/proto/src/from_proto.rs
@@ -890,7 +890,7 @@ pub fn parse_expr(
.iter()
.map(|e| parse_expr(e, registry))
.collect::<Result<Vec<_>, _>>()?,
- distinct: false, // TODO
+ distinct: expr.distinct,
})
}
ExprType::Alias(alias) => Ok(Expr::Alias(
diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs
index eecca1b6a..c843de630 100644
--- a/datafusion/proto/src/lib.rs
+++ b/datafusion/proto/src/lib.rs
@@ -912,6 +912,28 @@ mod roundtrip_tests {
roundtrip_expr_test(test_expr, ctx);
}
+ #[test]
+ fn roundtrip_count() {
+ let test_expr = Expr::AggregateFunction {
+ fun: AggregateFunction::Count,
+ args: vec![col("bananas")],
+ distinct: false,
+ };
+ let ctx = SessionContext::new();
+ roundtrip_expr_test(test_expr, ctx);
+ }
+
+ #[test]
+ fn roundtrip_count_distinct() {
+ let test_expr = Expr::AggregateFunction {
+ fun: AggregateFunction::Count,
+ args: vec![col("bananas")],
+ distinct: true,
+ };
+ let ctx = SessionContext::new();
+ roundtrip_expr_test(test_expr, ctx);
+ }
+
#[test]
fn roundtrip_approx_percentile_cont() {
let test_expr = Expr::AggregateFunction {
diff --git a/datafusion/proto/src/to_proto.rs b/datafusion/proto/src/to_proto.rs
index a022769dc..d3f68b3b4 100644
--- a/datafusion/proto/src/to_proto.rs
+++ b/datafusion/proto/src/to_proto.rs
@@ -502,7 +502,9 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode {
}
}
Expr::AggregateFunction {
- ref fun, ref args, ..
+ ref fun,
+ ref args,
+ ref distinct,
} => {
let aggr_function = match fun {
AggregateFunction::ApproxDistinct => {
@@ -550,6 +552,7 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode {
.iter()
.map(|v| v.try_into())
.collect::<Result<Vec<_>, _>>()?,
+ distinct: *distinct,
};
Self {
expr_type: Some(ExprType::AggregateExpr(aggregate_expr)),