Skip to content

Conversation

hequn8128
Copy link
Contributor

What is the purpose of the change

Iterate over entrysets instead of keys when we want to get both key and value. I went over the code in flink. Luckily, there are not many places we need to optimize.

Brief change log

  • Iterate over entrysets instead of keys when we want to get both key and value

Verifying this change

This change is already covered by existing tests, such as
OverWindowHarnessTest for changes in ProcTimeBoundedRangeOver
OverWindowITCase for changes in RowTimeBoundedRangeOver
GroupWindowITCase for changes in JavaUserDefinedAggFunctions
CorrelateITCase for changes in UserDefinedTableFunctions

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@fhueske
Copy link
Contributor

fhueske commented Feb 5, 2018

Thanks for the PR @hequn8128!
Changes look good, +1 to merge

@zentol
Copy link
Contributor

zentol commented Feb 5, 2018

merging.

zentol pushed a commit to zentol/flink that referenced this pull request Feb 5, 2018
zentol pushed a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol pushed a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol pushed a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol pushed a commit to zentol/flink that referenced this pull request Feb 6, 2018
@asfgit asfgit closed this in 5e41eaa Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants