Skip to content

fix(hydro_lang): move KeyedStream trait constraints from impl to methods#2319

Merged
shadaj merged 1 commit intohydro-project:mainfrom
sim-hash:keyed-stream-trait-constraints-2254
Nov 25, 2025
Merged

fix(hydro_lang): move KeyedStream trait constraints from impl to methods#2319
shadaj merged 1 commit intohydro-project:mainfrom
sim-hash:keyed-stream-trait-constraints-2254

Conversation

@sim-hash
Copy link
Contributor

@sim-hash sim-hash commented Nov 25, 2025

Moves K: Eq + Hash constraints from impl block where clauses to individual method where clauses in KeyedStream.

fixes #2254

@sim-hash sim-hash changed the title move KeyedStream trait constraints from impl to methods fix(hydro_lang): move KeyedStream trait constraints from impl to methods Nov 25, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the KeyedStream implementation to move K: Eq + Hash trait constraints from impl block where clauses to individual method where clauses. This improves API flexibility by allowing methods that don't require key hashing/equality (like map, filter, inspect) to work with any key type, while still enforcing constraints on methods that need to group by key (like aggregations and joins).

  • Removed K: Eq + Hash from four impl block where clauses
  • Added K: Eq + Hash (or K: Clone + Eq + Hash where needed) to 16 individual method where clauses
  • Methods requiring hashing/equality: scan, generator, fold_early_stop, first, various fold/reduce variants, value_counts, and filter_key_not_in

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MingweiSamuel
Copy link
Member

I believe not all of the methods will need Eq + Hash, e.g. maybe not pub fn first(self), @shadaj ?

@shadaj
Copy link
Member

shadaj commented Nov 25, 2025

I believe not all of the methods will need Eq + Hash, e.g. maybe not pub fn first(self), @shadaj ?

Actually first will require it as well, since it has to group by key to find the first per key. Will careful review of all the bounds though.

Copy link
Member

@shadaj shadaj left a comment

Choose a reason for hiding this comment

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

Looks good!

@shadaj shadaj merged commit 2b0c4d8 into hydro-project:main Nov 25, 2025
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move trait constraints on stream elements types from impl to function

3 participants

Comments