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.

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

[FLINK-6926] [table] Add support for SHA-224, SHA-384, SHA-512 #5324

Closed
wants to merge 2 commits into from

Conversation

genged
Copy link
Contributor

@genged genged commented Jan 20, 2018

What is the purpose of the change

This pull request implements SHA224, SHA384, SHA512 support in Flink SQL as discussed in FLINK-6926

Brief change log

  • Added SHA224, SHA384, SHA512 SQL functions
  • Added relevant unit tests

Verifying this change

This change added tests and can be verified as follows:

  • Added SQL expression tests
  • Added HashFunctionsTest with testAllApis
  • Validated both correct calculation and behavior for null input

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: don't know
  • The runtime per-record code paths (performance sensitive): don't know
  • 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? yes
  • If yes, how is the feature documented? docs

Copy link
Contributor

@twalthr twalthr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @genged. The overall code looks good, however, it seems that databases like MySQL make the bit length configurable (https://dev.mysql.com/doc/refman/5.5/en/encryption-functions.html#function_sha2). We should support SHA2(string, length) instead of SHA224(string) to be compliant with other SQL vendors.

@genged
Copy link
Contributor Author

genged commented Feb 7, 2018

@twalthr Added support for SHA2 as per your request. I also left support for using SHA224, SHA256, SHA384, SHA512 functions directly.

@twalthr
Copy link
Contributor

twalthr commented Apr 20, 2018

Thank you @genged. I will have a final pass over the code and merge this.

twalthr pushed a commit to twalthr/flink that referenced this pull request May 15, 2018
twalthr pushed a commit to twalthr/flink that referenced this pull request May 15, 2018
@asfgit asfgit closed this in d5de2bc May 15, 2018
sampathBhat pushed a commit to sampathBhat/flink that referenced this pull request Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants