ARROW-11221: [Rust] DF Implement GROUP BY support for Float32/Float64 #9175
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9175 +/- ##
==========================================
+ Coverage 81.55% 81.58% +0.02%
==========================================
Files 215 215
Lines 51600 51716 +116
==========================================
+ Hits 42084 42191 +107
- Misses 9516 9525 +9
Continue to review full report at Codecov.
|
|
No sure but I think it should be OK to use crate
|
There was a problem hiding this comment.
I agree with @mqy 's assessment that this is a relatively small new dependency (and it doesn't bring its own large depdneneyc stack https://github.com/reem/rust-ordered-float/blob/master/Cargo.toml) and so it should be fine.
ovr
force-pushed
the
issue-11221
branch
2 times, most recently
from
2769a30 to
bb098a9
Compare
| @@ -776,6 +785,14 @@ pub(crate) fn create_group_by_values( | |||
| for i in 0..group_by_keys.len() { | |||
| let col = &group_by_keys[i]; | |||
| match col.data_type() { | |||
| DataType::Float32 => { | |||
There was a problem hiding this comment.
I've updated PR, forget about this place. I am testing it now with real DB example.
There was a problem hiding this comment.
I suggest adding an end-to-end sql test in https://github.com/apache/arrow/blob/master/rust/datafusion/tests/sql.rs to make sure the plumbing is all hooked up correctly
There was a problem hiding this comment.
@alamb I've added tests, but I found a strange bug probably with count.
012c1ac#diff-4b06103cf2132b1ab297fbe8cd42622ecbe1109ea26df7d8b358fa36d739549cR357
Thanks
ovr
force-pushed
the
issue-11221
branch
2 times, most recently
from
8bd7523 to
012c1ac
Compare
Rust doesn't provide Eq, Hash for f32/f64 types inside stdlib, it's why I am using an external library called ordered-float which implements this traits. It's better to use external library instead of implementing own inside this repository. Closes apache#9175 from ovr/issue-11221 Authored-by: Dmitry Patsura <zaets28rus@gmail.com> Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Rust doesn't provide Eq, Hash for f32/f64 types inside stdlib, it's why I am using an external library called ordered-float which implements this traits. It's better to use external library instead of implementing own inside this repository.