-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add LuhnCheck function #4011
Add LuhnCheck function #4011
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/prestosql/cla. |
presto-main/src/test/java/io/prestosql/operator/scalar/TestLuhnCheckFunction.java
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/metadata/FunctionRegistry.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
1546895
to
381b35a
Compare
b0e14dd
to
b3dc066
Compare
@electrum are we ready to remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation for this in presto-docs/src/main/sphinx/functions/string.rst
(String Functions doesn't seem like the right place to document this, but I can't find a better spot)
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
We may ultimately want a validation functions library. There are a whole host of them you could write. But not worth it for just this one. |
b3dc066
to
2fc023f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martint thoughts on the name?
presto-main/src/main/java/io/prestosql/operator/scalar/LuhnCheckFunction.java
Outdated
Show resolved
Hide resolved
presto-main/src/test/java/io/prestosql/operator/scalar/TestLuhnCheckFunction.java
Show resolved
Hide resolved
I think |
6335674
to
18c590f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please rebase and I'll merge this.
18c590f
to
5b36bd0
Compare
5b36bd0
to
77eb488
Compare
Can you update the commit message to something like
Easy way is to run |
77eb488
to
7b82a38
Compare
Thanks! |
Add builtin in credit card validation function - LuhnCheckFunction