You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/06/16 09:34:52 UTC

[GitHub] [flink] deadwind4 opened a new pull request, #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

deadwind4 opened a new pull request, #19988:
URL: https://github.com/apache/flink/pull/19988

   ## What is the purpose of the change
   
   Support ASCII and CHR built-in function in the Table API
   
   ## Brief change log
   
     - *Support ASCII in the Table API*
     - *Support CHR*
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
     - *Added test that validates that ASCII is effective in the Table API.*
     - *Added test that validates that CHR is effective in the Table API.*
   
   ## 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)`: (yes)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (docs)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] snuyanzin commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1158538968

   It looks like the implementation is going through the old stack api which is deprecated...
   It is better to use new stack api available after FLIP-32 and FLIP-65. As an example you can have a look at implementation of `IFNULL` or `TYPEOF` made within https://issues.apache.org/jira/browse/FLINK-20522


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] deadwind4 commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1160014687

   @dianfu I agree with you. If we migrate all built-in functions to the new stack, we could create a new Jira ticket to target this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] deadwind4 commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1158459654

   @dianfu Please review it, thanks a lot.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dianfu commented on a diff in pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
dianfu commented on code in PR #19988:
URL: https://github.com/apache/flink/pull/19988#discussion_r904635241


##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -1010,6 +1012,16 @@ public OutType toBase64() {
         return toApiSpecificExpression(unresolvedCall(TO_BASE64, toExpr()));
     }
 
+    /** Returns the numeric value of the first character the input string. */

Review Comment:
   ```suggestion
       /** Returns the numeric value of the first character of the input string. */
   ```



##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -1010,6 +1012,16 @@ public OutType toBase64() {
         return toApiSpecificExpression(unresolvedCall(TO_BASE64, toExpr()));
     }
 
+    /** Returns the numeric value of the first character the input string. */
+    public OutType ascii() {
+        return toApiSpecificExpression(unresolvedCall(ASCII, toExpr()));
+    }
+
+    /** Returns the ASCII character result of the input string. */
+    public OutType chr() {

Review Comment:
   Should also update expression.py



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] deadwind4 commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1158541318

   @snuyanzin  Thanks a lot, I will research the implementation of `IFNULL` and `TYPEOF`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dianfu commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
dianfu commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1169444531

   LGTM. Merging... 
   
   >  If we migrate all built-in functions to the new stack, we could create a new Jira ticket to target this.
   Agree. Let's do that in a separate ticket if we found it necessary.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dianfu commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
dianfu commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1160011713

   @snuyanzin @deadwind4 Per my understanding, the new stack is to ease the adding of new built-in functions. For existing functions, as the work is already done, it isn't convincing for me to migrate to the new stack as it means more workload which seems deviating from the original intention of introducing the new stack. However, if there are plans to migrate all built-in functions to the new stack, then it's another story.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dianfu closed pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
dianfu closed pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API
URL: https://github.com/apache/flink/pull/19988


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] flinkbot commented on pull request #19988: [FLINK-28092][table] Support ASCII and CHR built-in function in the Table API

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19988:
URL: https://github.com/apache/flink/pull/19988#issuecomment-1157453000

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "8a80434b58412b50f1e32c9823191cb5d2b5bc54",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8a80434b58412b50f1e32c9823191cb5d2b5bc54",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8a80434b58412b50f1e32c9823191cb5d2b5bc54 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org