[GitHub] flink issue #6253: [FLINK-8094][Table API & SQL] Support other types for Exi...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6253 @xccui Yes. Without defining expression with custom reserved keyword, we may need to leverage function which would be non-UDF which means we need to add the function to default provided function

[GitHub] flink issue #6252: [FLINK-9742][Table API & SQL] Expose Expression.resultTyp...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6252 Addressed. ---

[GitHub] flink pull request #6252: [FLINK-9742][Table API & SQL] Expose Expression.re...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/flink/pull/6252#discussion_r200352236 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionUtils.scala --- @@ -22,13 +22,16 @@ import java.lang

[GitHub] flink issue #6253: [FLINK-8094][Table API & SQL] Support other types for Exi...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6253 Btw, looks like it may be also possible for ExistingField to replace needs of ParsingExistingField, via adding optional dateformat parameter (not sure which is more preferred), but I feel we can

[GitHub] flink issue #6253: [FLINK-8094][Table API & SQL] Support other types for Exi...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6253 @fhueske Thanks for the nice feedback! Addressed your comment except regarding unit test. Let me try out adding unit test later, since it might bring too high barrier as of now. ---

[GitHub] flink issue #6252: [FLINK-9742][Table API & SQL] Expose Expression.resultTyp...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6252 @fhueske Addressed. Please take a look again. ---

[GitHub] flink issue #6252: [FLINK-9742][Table API & SQL] Widen scope of Expression.r...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6252 @fhueske Ah I didn't notice the object is already existing. Thanks for letting me know! ---

[GitHub] flink issue #6252: [FLINK-9742][Table API & SQL] Widen scope of Expression.r...

2018-07-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6252 @fhueske Yeah, that would be great. I'm happy to revert the change and introduce new class. I'm just not sure where to place and how to name it, since it will be going to have

[GitHub] flink issue #6253: [WIP][FLINK-8094][Table API & SQL] Support other types fo...

2018-07-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6253 If we would like to answer 1 as "Yes, apply the change to ExistingField.", we can rebase this PR with below branch. https://github.com/HeartSaVioR/flink/commits/FLINK-8

[GitHub] flink issue #6256: [MINOR] Add generated avro java codes to RAT exclusion li...

2018-07-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6256 @zentol Addressed review comment. ---

[GitHub] flink issue #6256: [MINOR] Add generated avro java codes to .ignore to avoid...

2018-07-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6256 @zentol First of all, thanks for the reviewing! The RAT plugin `reads and parses` the .gitignore file like below so adding it to .gitignore file in root directory is effective. The issue

[GitHub] flink issue #6256: [MINOR] Add generated avro java codes to .ignore to avoid...

2018-07-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6256 There's already .gitignore file in flink-confluent-schema-registry module directory, but it doesn't look like effective while running `mvn clean verify` in flink root directory. ---

[GitHub] flink pull request #6256: [MINOR] Add generated avro java codes to .ignore t...

2018-07-04 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/flink/pull/6256 [MINOR] Add generated avro java codes to .ignore to avoid RAT error ## What is the purpose of the change This patch adds generated avro java codes to .ignore to avoid RAT error when

[GitHub] flink issue #6253: [WIP][FLINK-8094][Table API & SQL] Support other types fo...

2018-07-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/flink/pull/6253 Seeking for some guides on this PR: 1. Maybe we could replace the implementation of ExistingField with the implementation of IsoDateStringAwareExistingField since the implementation

[GitHub] flink pull request #6253: [FLINK-8094][Table API & SQL] Support other types ...

2018-07-04 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/flink/pull/6253 [FLINK-8094][Table API & SQL] Support other types for ExistingField rowtime extractor ## What is the purpose of the change This patch proposes new rowtime extractor which is impr

[GitHub] flink pull request #6252: [FLINK-9742][Table API & SQL] Widen scope of Expre...

2018-07-04 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/flink/pull/6252 [FLINK-9742][Table API & SQL] Widen scope of Expression.resultType to 'public' ## What is the purpose of the change This patch proposes widen scope of Expression.resultType to 'pu