Skip to content

Conversation

@tustvold
Copy link
Contributor

Which issue does this PR close?

Closes #364.

Rationale for this change

Allows comparing GroupByScalar values

What changes are included in this PR?

Adds PartialOrd and Ord derives to GroupByScalar

Are there any user-facing changes?

No, only changes a crate-local. There didn't seem to be consensus on the ticket as to whether I should make this enum public

Signed-off-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>
@alamb alamb added datafusion enhancement New feature or request labels May 21, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #368 (83c0467) into master (5c5a0bb) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   75.86%   75.88%   +0.01%     
==========================================
  Files         143      143              
  Lines       23758    23773      +15     
==========================================
+ Hits        18025    18041      +16     
+ Misses       5733     5732       -1     
Impacted Files Coverage Δ
datafusion/src/physical_plan/group_scalar.rs 65.88% <ø> (ø)
datafusion/src/physical_plan/mod.rs 84.70% <0.00%> (-2.36%) ⬇️
datafusion/tests/sql.rs 99.89% <0.00%> (+<0.01%) ⬆️
datafusion/src/physical_plan/hash_join.rs 86.05% <0.00%> (+0.47%) ⬆️
datafusion/src/physical_plan/display.rs 86.66% <0.00%> (+2.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c5a0bb...83c0467. Read the comment docs.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes total sense to me. 👍

@Dandandan Dandandan merged commit 76ac914 into apache:master May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Compare to GroupByScalar

4 participants