Skip to content

Conversation

@dianfu
Copy link
Contributor

@dianfu dianfu commented Dec 12, 2018

What is the purpose of the change

This pull request fix the merge logic of DISTINCT aggregates.

Brief change log

  • Fix the codegen logic in AggregationCodeGenerator to extract the distinct fields for merge

Verifying this change

This change added tests and can be verified as follows:

  • Updated integration tests SqlITCase#testDistinctAggWithMergeOnEventTimeSessionGroupWindow

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)
  • If yes, how is the feature documented? (not applicable)

@fhueske
Copy link
Contributor

fhueske commented Dec 13, 2018

Thanks for fixing this bug @dianfu!

Will merge this fix.

@asfgit asfgit closed this in ed0aefa Dec 13, 2018
@dianfu
Copy link
Contributor Author

dianfu commented Dec 13, 2018

Thanks a lot for the review and merge @fhueske !

asfgit pushed a commit that referenced this pull request Dec 13, 2018
asfgit pushed a commit that referenced this pull request Dec 13, 2018
JTaky pushed a commit to JTaky/flink that referenced this pull request Dec 26, 2018
ashangit pushed a commit to criteo-forks/flink that referenced this pull request Dec 31, 2018
tisonkun pushed a commit to tisonkun/flink that referenced this pull request Jan 17, 2019
@dianfu dianfu deleted the fix_distinct branch June 10, 2020 02:55
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.

4 participants