[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205960985
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
Cool, it works now.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205959747
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
Never mind. You can update the pr with the code which you think it is right
and I will take another look.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205959687
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
I tested with `TINYINT `, `SMALLINT` and `BIGINT` but all failed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205959590
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
It's weird. I checkout your latest branch just now and find the exception
still be thrown. Could you add the tests into the PR? Thanks a lot.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205959590
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
It's weird. I checkout your latest branch just now and find the exception
still be thrown.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205959590
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
It's weird. I checkout your latest branch and find the exception still will
be thrown.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205938013
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
@yanghua Sorry that haven't make it clear.
Yes, what i mean is add multi-functions. You probably have to add the
following changes:
1. add different functions in `ScalarFunctions.scala`.
2. add other `addSqlFunctionMethod` in `FunctionGenerator`.
```
addSqlFunctionMethod(
CHR,
Seq(SHORT_TYPE_INFO),
STRING_TYPE_INFO,
BuiltInMethods.CHR
)
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205938013
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
@yanghua Sorry that haven't make it clear.
Yes, what i mean is add multi-functions. You probably have to add the
following changes:
1. add different functions in `ScalarFunctions.scala`.
2. add other `addSqlFunctionMethod` in `FunctionGenerator`.
```
addSqlFunctionMethod(
CHR,
Seq(LONG_TYPE_INFO),
STRING_TYPE_INFO,
BuiltInMethods.CHR
)
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932698
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
Review comment:
For the function signature, I think we should support other types, say
Byte/Short/Long. Exceptions will be thrown if we write `chr(CAST(1 AS
SMALLINT))`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932701
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
+* If the ASCII less then 0 or greater than 255, return null.
+*/
+ def chr(ascii: Integer): String = {
+if (ascii < 0 || ascii > 255) {
Review comment:
do null check
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932695
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
##
@@ -544,11 +544,22 @@ trait ImplicitExpressionOperations {
def overlay(newString: Expression, starting: Expression, length: Expression)
=
Overlay(expr, newString, starting, length)
+ /**
+* Returns the numeric value of the leftmost character of the string.
+*/
+ def ascii() = Ascii(expr)
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64() = FromBase64(expr)
+ /*
+* Returns string contains a character which converts from a ASCII integer.
Review comment:
Returns the character corresponding to the input ASCII integer. If the
ASCII code less then 0 or greater than 255, returns null.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932693
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
##
@@ -544,11 +544,22 @@ trait ImplicitExpressionOperations {
def overlay(newString: Expression, starting: Expression, length: Expression)
=
Overlay(expr, newString, starting, length)
+ /**
+* Returns the numeric value of the leftmost character of the string.
+*/
+ def ascii() = Ascii(expr)
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64() = FromBase64(expr)
+ /*
Review comment:
/* => /**
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932686
##
File path: docs/dev/table/tableApi.md
##
@@ -2507,6 +2518,17 @@ concat_ws(separator, string1, string2,...)
Returns the string that results from concatenating the arguments
using a separator. The separator is added between the strings to be
concatenated. Returns NULL If the separator is NULL. concat_ws() does not skip
empty strings. However, it does skip any NULL argument. E.g.
concat_ws("~", "AA", "BB", "", "CC") returns
AA~BB~~CC
+
+
+
+{% highlight text %}
+INTEGER.chr()
+{% endhighlight %}
+
+
+Returns string contains a character which converts from a ASCII
integer. If the ASCII less then 0 or greater than 255, return null. E.g.
97.chr() returns a
Review comment:
1. return null. => returns null.
2. returns a => returns 'a'
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932661
##
File path: docs/dev/table/sql.md
##
@@ -1850,6 +1860,17 @@ FROM_BASE64(text string)
+
+
+{% highlight text %}
+CHR(ascii integer)
+{% endhighlight %}
+
+
+Returns string contains a character which converts from a ASCII
integer. If the ASCII less then 0 or greater than 255, return null. E.g.
CHR(65) returns A, CHR(97) returns
a.
Review comment:
1. Returns string contains a character which converts from a ASCII integer.
=> Returns the character corresponding to the input ASCII integer.
2. If the ASCII => If the ASCII code
3. return null. => returns null.
4. returns a => returns 'a'
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932678
##
File path: docs/dev/table/tableApi.md
##
@@ -2507,6 +2518,17 @@ concat_ws(separator, string1, string2,...)
Returns the string that results from concatenating the arguments
using a separator. The separator is added between the strings to be
concatenated. Returns NULL If the separator is NULL. concat_ws() does not skip
empty strings. However, it does skip any NULL argument. E.g.
concat_ws("~", "AA", "BB", "", "CC") returns
AA~BB~~CC
+
+
+
+{% highlight text %}
Review comment:
1. {% highlight text %} => {% highlight java %}
2. add doc for scala
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add ASCII/CHR function for table/sql API
hequn8128 commented on a change in pull request #6432: [FLINK-9970] Add
ASCII/CHR function for table/sql API
URL: https://github.com/apache/flink/pull/6432#discussion_r205932697
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/functions/ScalarFunctions.scala
##
@@ -196,9 +196,35 @@ object ScalarFunctions {
new String(data)
}
+ /**
+* Returns the numeric value of the leftmost character of the string str.
+* Returns 0 if str is the empty string. Returns NULL if str is NULL.
+*/
+ def ascii(str: String): String = {
+if (str == null) {
+ null
+} else if (str.equals("")) {
+ ""
+} else {
+ str.charAt(0).toByte.toString
+}
+ }
+
/**
* Returns the base string decoded with base64.
*/
def fromBase64(str: String): String = new String(Base64.decodeBase64(str))
+ /**
+* Returns string contains a character which converts from a ASCII integer.
Review comment:
Returns the character corresponding to the input ASCII integer. If the ASCII
less then 0 or greater than 255, returns null.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
