Skip to content

Conversation

@brancz
Copy link
Contributor

@brancz brancz commented Jan 14, 2026

Which issue does this PR close?

Implements the hashing part of #19782

Rationale for this change

To aggregate by ListView arrays, we need to be able to hash them.

What changes are included in this PR?

The hashing code and tests.

Are these changes tested?

Yes, unit tests.

Are there any user-facing changes?

No, only additive functionality.

@alamb @Jefffrey

@github-actions github-actions bot added the common Related to common crate label Jan 14, 2026
Comment on lines +1220 to +1221
assert_eq!(hashes[0], hashes[5]); // same content [0, 1, 2]
assert_eq!(hashes[1], hashes[4]); // both null
assert_eq!(hashes[2], hashes[3]); // same content [3, null, 5]
assert_eq!(hashes[1], hashes[6]); // null vs empty list
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps some negative tests to ensure hashes differ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you be more specific what you mean? are you just asking to test that eg. hash 0 and 2 are different?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@brancz
Copy link
Contributor Author

brancz commented Jan 16, 2026

We good to merge this?

@alamb
Copy link
Contributor

alamb commented Jan 16, 2026

I took a look at this PR and it looks good to me too -- thanks @brancz @martin-g and @Jefffrey

@alamb alamb added this pull request to the merge queue Jan 16, 2026
Merged via the queue into apache:main with commit 3d90d4b Jan 16, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants