You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/07 20:18:40 UTC
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6581: feat: `DISTINCT` bitwise and boolean aggregate functions
alamb commented on code in PR #6581:
URL: https://github.com/apache/arrow-datafusion/pull/6581#discussion_r1222110398
##########
datafusion/core/tests/sqllogictests/test_files/aggregate.slt:
##########
@@ -99,6 +99,18 @@ SELECT bit_xor(c5), bit_xor(c6), bit_xor(c7), bit_xor(c8), bit_xor(c9) FROM aggr
----
1632751011 5960911605712039654 148 54789 169634700
+# csv_query_bit_xor_distinct
+query IIIII
+SELECT bit_xor(distinct c5), bit_xor(distinct c6), bit_xor(distinct c7), bit_xor(distinct c8), bit_xor(distinct c9) FROM aggregate_test_100
+----
+1632751011 5960911605712039654 196 54789 169634700
+
+# csv_query_bit_xor_distinct_expr
+query I
+SELECT bit_xor(distinct c5 % 2) FROM aggregate_test_100
+----
+-2
+
Review Comment:
even though, as you note,`BIT_AND(DISTINCT), BIT_OR(DISTINCT), BOOL_AND(DISTINCT) and BOOL_OR(DISTINCT)` are the same as their non distinct versions, I think we should still add SQL coverage so that we don't accidentally break the feature during some future refactoring
##########
datafusion/physical-expr/src/aggregate/bit_and_or_xor.rs:
##########
@@ -751,6 +753,170 @@ impl RowAccumulator for BitXorRowAccumulator {
}
}
+/// Expression for a BIT_XOR(DISTINCT) aggregation.
+#[derive(Debug, Clone)]
+pub struct DistinctBitXor {
Review Comment:
I wonder if it is possible to avoid duplicated code for distinct aggregates -- they are all basically doing the same thing 🤔
##########
datafusion/core/tests/sqllogictests/test_files/aggregate.slt:
##########
@@ -99,6 +99,18 @@ SELECT bit_xor(c5), bit_xor(c6), bit_xor(c7), bit_xor(c8), bit_xor(c9) FROM aggr
----
1632751011 5960911605712039654 148 54789 169634700
+# csv_query_bit_xor_distinct
Review Comment:
```suggestion
# csv_query_bit_xor_distinct (should be different than above)
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org