Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hash for StructField should consider more than the name #2045

Closed
rtyler opened this issue Jan 7, 2024 · 3 comments · Fixed by #2190
Closed

Hash for StructField should consider more than the name #2045

rtyler opened this issue Jan 7, 2024 · 3 comments · Fixed by #2190
Assignees
@sonhmai
Labels
binding/rust Issues for the Rust crate bug Something isn't working
Milestone
Rust v1.0.0

Comments

@rtyler
Copy link
Member

rtyler commented Jan 7, 2024

Two StructFields with the same name but different data_types are considered to hash equivalently, which I think is incorrect.

I'm working around this for now, but filing for @roeap and I to discuss later
@rtyler rtyler added bug Something isn't working binding/rust Issues for the Rust crate labels Jan 7, 2024
@roeap
Copy link
Collaborator

roeap commented Jan 27, 2024

Agreed, in fact, name, data_type and nullability should match for a field to be considered equivalent.

@rtyler rtyler added this to the Rust v0.18 milestone Feb 6, 2024
@sonhmai
Copy link
Contributor

sonhmai commented Feb 18, 2024

take

sonhmai added a commit to sonhmai/delta-rs that referenced this issue Feb 18, 2024
@sonhmai
fix: add data_type and nullable to StructField hash (delta-io#2045)
dafb02b
@sonhmai sonhmai mentioned this issue Feb 18, 2024
Merged
@sonhmai
Copy link
Contributor

sonhmai commented Feb 18, 2024 •
edited
Loading

@rtyler: I added PR #2190 to fix this.

ion-elgreco pushed a commit that referenced this issue Feb 24, 2024
@sonhmai @rtyler
fix: add data_type and nullable to StructField hash (#2045) (#2190)
bcf124c 
# Description
Correct hashing of StructField and add tests.
- Current: Two StructFields with the same name but different `data_type`
or different `nullable` are considered to hash equivalently.
- To be: `name`, `data_type` and `nullability` should match for a field
to be considered equivalent.

# Related Issue(s)
closes #2045

---------

Co-authored-by: R. Tyler Croy <rtyler@brokenco.de>
Co-authored-by: sonhmai <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sonhmai sonhmai

Labels
binding/rust Issues for the Rust crate bug Something isn't working
Projects
None yet
Milestone
Rust v1.0.0
Development

Successfully merging a pull request may close this issue.

fix: add data_type and nullable to StructField hash (#2045) sonhmai/delta-rs
3 participants
@rtyler @sonhmai @roeap