-
Notifications
You must be signed in to change notification settings - Fork 13k
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-34108][table] Add built-in URL_ENCODE and URL_DECODE function. #24773
base: master
Are you sure you want to change the base?
Conversation
@flinkbot run azure |
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlDecodeFunction.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlEncodeFunction.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlDecodeFunction.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlDecodeFunction.java
Show resolved
Hide resolved
...table-planner/src/test/java/org/apache/flink/table/planner/functions/UrlFunctionsITCase.java
Outdated
Show resolved
Hide resolved
...table-planner/src/test/java/org/apache/flink/table/planner/functions/UrlFunctionsITCase.java
Outdated
Show resolved
Hide resolved
Thanks for your contribution @superdiaodiao , also ci is failed and i tend to think it is related to your changes in python code, could you please have a look? |
Thanks for your review, it helps a lot. |
@flinkbot run azure |
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlDecodeFunction.java
Outdated
Show resolved
Hide resolved
...table-planner/src/test/java/org/apache/flink/table/planner/functions/UrlFunctionsITCase.java
Show resolved
Hide resolved
...table-planner/src/test/java/org/apache/flink/table/planner/functions/UrlFunctionsITCase.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlDecodeFunction.java
Outdated
Show resolved
Hide resolved
...table-planner/src/test/java/org/apache/flink/table/planner/functions/UrlFunctionsITCase.java
Outdated
Show resolved
Hide resolved
...runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/UrlEncodeFunction.java
Outdated
Show resolved
Hide resolved
@snuyanzin @MartijnVisser @HuangXingBo PLZ take a look~ |
try { | ||
return StringData.fromString(URLDecoder.decode(value.toString(), charset.name())); | ||
} catch (UnsupportedEncodingException | RuntimeException e) { | ||
return null; |
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 think we should log the exception here, with the log containing the value.toString(), charset.name() and Exception type and message. This mould mean that the logs could be used to diagnose the cause of the issue.
try { | ||
return StringData.fromString(URLEncoder.encode(url.toString(), charset.name())); | ||
} catch (UnsupportedEncodingException e) { | ||
return null; |
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 suggest logging the exception here.
def url_decode(self) -> 'Expression[str]': | ||
""" | ||
Decodes a given string in 'application/x-www-form-urlencoded' format using the UTF-8 | ||
encoding scheme. If input is null, or there is an issue with the decoding process(such |
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.
nit: input -> the input
def url_encode(self) -> 'Expression[str]': | ||
""" | ||
Translates a string into 'application/x-www-form-urlencoded' format using the UTF-8 | ||
encoding scheme. If input is null, or there is an issue with the encoding process, or |
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.
nit: input -> the input
What is the purpose of the change
issue: https://issues.apache.org/jira/browse/FLINK-34108
This is an implementation of URL_ENCODE and URL_DECODE
Brief change log
Syntax:
url_encode(url)
Arguments:
url: a string represents a URL
Returns:
translates a string into 'application/x-www-form-urlencoded' format using a specific encoding scheme(UTF-8), will be null if input is null or encode failed.
Examples:
Syntax:
url_decode(value)
Arguments:
value: a URL encoded
Returns:
decodes a string in 'application/x-www-form-urlencoded' format using a specific encoding scheme(UTF-8), will be null if input is null or decode failed.
Examples:
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation