Skip to content

implement Hash for various types and replace PartialOrd - #1580

Merged
jimexist merged 3 commits into
masterfrom
impl-hash-for-various-types
Jan 16, 2022
Merged

implement Hash for various types and replace PartialOrd#1580
jimexist merged 3 commits into
masterfrom
impl-hash-for-various-types

Conversation

@jimexist

@jimexistjimexist commented Jan 16, 2022

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

Cherry-pick some changes from #1556 to reduce future merge overhead

What changes are included in this PR?

Are there any user-facing changes?

@jimexist
jimexist requested a review from houqpJanuary 16, 2022 03:12
@jimexistjimexist changed the title implement Hash for various typesimplement Hash for various types and replace PartialOrdJan 16, 2022

@houqphouqp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @jimexist for porting this!

@houqphouqp added the api change Changes the API exposed to users of the crate label Jan 16, 2022
@houqp

Copy link
Copy Markdown
Member

For the context, this is because arrow2's datatype doesn't derive PartialOrd anymore because order between types is not defined semantically.

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me

Wildcard,
}

/// Fixed seed for the hashing so that Ords are consistent across runs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍


impl Hash for WindowFrameBound {
fn hash<H: Hasher>(&self, state: &mut H) {
self.get_rank().hash(state)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@jimexist
jimexist merged commit 6f7b2d2 into masterJan 16, 2022
@jimexist
jimexist deleted the impl-hash-for-various-types branch January 16, 2022 12:59
@jimexistjimexist added this to the arrow2 milestone Jan 16, 2022
@alambalamb changed the title implement Hash for various types and replace PartialOrdimplement Hash for various types and replace PartialOrdFeb 10, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api changeChanges the API exposed to users of the crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jimexist@houqp@alamb