-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-8238][SPARK-8239][SPARK-8242][SPARK-8243][SPARK-8268][SQL]Add ascii/base64/unbase64/encode/decode functions #6843
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
Conversation
|
Test build #34986 has finished for PR 6843 at commit
|
|
@adrian-wang @zhichao-li can you review the code for me? |
|
Test build #34990 has finished for PR 6843 at commit
|
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.
Upper case these all?
|
Test build #35132 has finished for PR 6843 at commit
|
|
@rxin, any more comments on this? |
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.
i don't think this is necessary since it is a case class?
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.
btw i don't think we should be making everything uppercase..
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.
Probably the registered function name will make more sense, than giving the user a class name. What do you think?
|
Test build #35522 has finished for PR 6843 at commit
|
a5e89b6 to
da6a788
Compare
|
Test build #36330 has finished for PR 6843 at commit
|
da6a788 to
947a88a
Compare
|
Test build #36341 has finished for PR 6843 at commit
|
947a88a to
9d6f9f4
Compare
|
Test build #36500 has finished for PR 6843 at commit
|
|
Test build #36501 has finished for PR 6843 at commit
|
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.
can you make this extend BinaryExpression? You can just define def bin = left, and def charset = right.
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.
Actually that's my intention, as I think the parameters is asymmetric semantically. Not sure if you are thinking the code impovement like #7157?
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.
remove "As of Hive 0.12.0"
|
I'm going to merge this first. Please submit a cleanup pr. |
This is a the follow up of #6843. Author: Cheng Hao <[email protected]> Closes #7230 from chenghao-intel/str_funcs2_followup and squashes the following commits: 52cc553 [Cheng Hao] update the code as comment
Add
ascii,base64,unbase64,encodeanddecodeexpressions.