[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-17 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-219902602 @fhueske @twalthr thank you for your reviews and detailed comments through this issue 👍 . I'm quite excited to have it merged :) --- If your project is set up for

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-17 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/1958 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-17 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-219812166 Merging this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-17 Thread twalthr
Github user twalthr commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-219650388 @fhueske I looked through the changes. You have my +1 to merge. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-13 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-219085233 Hi @yjshen, thanks a lot for the update! This PR is good to merge, IMO. @twalthr, let me know if you want to have another look as well. Otherwise, I'll merge this

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-13 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r63206959 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/comparison.scala --- @@ -74,6 +109,8 @@ case class IsNull(child:

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-13 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r63180341 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/comparison.scala --- @@ -74,6 +109,8 @@ case class IsNull(child:

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-13 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r63177787 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/comparison.scala --- @@ -74,6 +109,8 @@ case class IsNull(child:

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218511092 @fhueske @twalthr thanks for the review work! I've updated the code just now. Look forward to more feedbacks on this :) --- If your project is set up for it, you

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218475561 @yjshen great work! PR looks very good. I had only some minor refactoring comments. --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218477620 Hi @twalthr, thanks very much for the review work! I'll resolve your comments shortly :) --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62856490 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/UnresolvedException.scala --- @@ -15,9 +15,6 @@ * See the

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62855247 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/call.scala --- @@ -18,85 +18,26 @@ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62853599 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/scala/table/expressionDsl.scala --- @@ -130,19 +130,17 @@ trait

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62851933 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/call.scala --- @@ -18,85 +18,26 @@ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62853748 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/TypeCoercion.scala --- @@ -0,0 +1,64 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62852367 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/cast.scala --- @@ -20,19 +20,55 @@ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62851129 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/call.scala --- @@ -18,85 +18,26 @@ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62849325 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/UnresolvedException.scala --- @@ -15,9 +15,6 @@ * See the

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62848946 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/TypeCoercion.scala --- @@ -0,0 +1,64 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread twalthr
Github user twalthr commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62843680 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/scala/table/expressionDsl.scala --- @@ -130,19 +130,17 @@ trait

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-11 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218447948 Hi @fhueske , I've just finished my work, can you give a another pass of review? Thanks! --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62745064 --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/api/scala/expression/ScalarFunctionsTest.scala --- @@ -215,7 +215,7 @@ class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218278581 Hi @fhueske, part of the comments are resolved and updated, two TODOs: - [ ] Type coercion for expressions, for example: - `Add(int, long)` should be auto

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62742492 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62742390 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62731427 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/StreamTableEnvironment.scala --- @@ -92,8 +93,7 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62715643 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62715347 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/StreamTableEnvironment.scala --- @@ -92,8 +93,7 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62714636 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62711917 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62709108 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62708060 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62705691 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62703535 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -152,13 +160,13 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62703065 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -152,13 +160,13 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62685650 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/validate/exceptions.scala --- @@ -0,0 +1,20 @@ +/* + * Licensed to

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62680102 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -197,6 +209,17 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-218182019 Hi @yjshen, sorry for the time it took to review the PR. I like the approach and the PR looks good, IMO. I had a few minor comments and questions. Let me know what you

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62683790 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62683921 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62685356 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/validate/FunctionCatalog.scala --- @@ -0,0 +1,159 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62685176 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/table.scala --- @@ -472,61 +339,39 @@ class Table(

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62684793 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/table.scala --- @@ -472,61 +339,39 @@ class Table(

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62683416 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62684644 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62684422 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62684054 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62683621 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,309 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62681558 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/arithmetic.scala --- @@ -25,15 +25,29 @@ import

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62683029 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62682986 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/LogicalNode.scala --- @@ -0,0 +1,169 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62682619 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/mathExpressions.scala --- @@ -0,0 +1,147 @@ +/* + *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62682327 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/logic.scala --- @@ -21,25 +21,47 @@ import

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62682155 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/fieldExpression.scala --- @@ -20,27 +20,98 @@ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62681920 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/arithmetic.scala --- @@ -25,15 +25,29 @@ import

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62681359 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/aggregations.scala --- @@ -36,41 +39,59 @@ abstract sealed class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62681024 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/Expression.scala --- @@ -17,13 +17,34 @@ */ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62680699 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -152,13 +160,13 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-10 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1958#discussion_r62679391 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/StreamTableEnvironment.scala --- @@ -92,8 +93,7 @@ abstract class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-09 Thread twalthr
Github user twalthr commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-217862197 Yes, I will also take a look at it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-06 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-217430461 I took a brief look at the PR. The overall structure of the validation looks good to me. Will do a more in-depth review in the next days. @twalthr, can you have another

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-216506763 That's a known issue, see FLINK-3860. No need to worry about this PR. I'll have a look soon, thanks for the update! --- If your project is set up for it, you can

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-216505284 Test failure due to irrelevant test: ``` [INFO] flink-table SUCCESS [08:03 min] [INFO]

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-216502690 @zentol OK, I've closed it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread yjshen
Github user yjshen closed the pull request at: https://github.com/apache/flink/pull/1916 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-216501884 Since this PR is a substitute, could you close the old one? Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1958#issuecomment-216487312 This PR substitute #1916 by squashing several previous commits into single one for easier rebase. @fhueske I've implemented eager validation here, can you take a

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-03 Thread yjshen
GitHub user yjshen opened a pull request: https://github.com/apache/flink/pull/1958 [FLINK-3754][Table]Add a validation phase before construct RelNode using TableAPI This PR aims at adding an extra phase of **validation** for plans generated from Table API, matches the

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-02 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-216270686 Great, thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-05-02 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-216269369 Will start to work on eager validation now. :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215422057 @fhueske If that's the preferred way, I will try to do some simplifications and see. --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215414605 If we decide for eager validation, we can also simplify the code a bit, right? I might be wrong but we could remove `PlanPreparation` and the recursive

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215336755 Hi @fhueske @twalthr, thanks for the detailed review work! 👍 I've left some comment above to express my opinion, looking forward to more discussions on this :)

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215334972 @twalthr, translate table API call to `SqlNode` and use `validator` in Calcite seems another reasonable solution, maybe it's hard to do `eager validation` as @fhueske

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61382801 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -355,4 +380,26 @@ object TableEnvironment {

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215332571 > I think it would be good to eagerly check each method call of the Table API. This would make debugging easier, because exceptions would be thrown where the error is

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-28 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61378731 --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/api/table/test/ScalarFunctionsTest.scala --- @@ -215,7 +215,7 @@ class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215142967 Hi @yjshen, thanks for your patience. I also finished a first pass over the PR. I'd like to propose a third alternative, in addition to the custom validation

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61291959 --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/api/table/test/ScalarFunctionsTest.scala --- @@ -215,7 +215,7 @@ class

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61291865 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/Expression.scala --- @@ -17,13 +17,34 @@ */ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61291698 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/expressions/Expression.scala --- @@ -17,13 +17,34 @@ */ package

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread fhueske
Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r61291621 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/TableEnvironment.scala --- @@ -355,4 +380,26 @@ object TableEnvironment {

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-27 Thread twalthr
Github user twalthr commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-215061267 @yjshen I looked through the code changes. I was quite impressed that your PR touches nearly every class of the current API. Basically your changes seem to

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-25 Thread twalthr
Github user twalthr commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-214332719 Thanks for the contribution @yjshen. I will also have a look at it tomorrow. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-24 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-213900827 @fhueske, thanks for your job. Looking forward to more discussions on this :) --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-22 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-213567820 Thanks for the additional information @yjshen! I'm a bit behind with PR reviews, but will definitely have a look begin of next week. Thanks, Fabian --- If your project

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-22 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-213419043 The type annotation work is done from bottom to top: Firstly, we know each schema of the two input, and we know `List[] expression` in `Project` are used to

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-22 Thread yjshen
Github user yjshen commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-213411425 For ease of review, I would like to explain this RP with more details. While table api are called to construct a query, it was first constructed as a operator

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-22 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r60728611 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/logical/operators.scala --- @@ -0,0 +1,188 @@ +/* + * Licensed

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-22 Thread yjshen
Github user yjshen commented on a diff in the pull request: https://github.com/apache/flink/pull/1916#discussion_r60728312 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/table.scala --- @@ -199,12 +151,12 @@ class Table( *

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-20 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1916#issuecomment-212485745 Thanks @yjshen for working on this issue! Unified validation and exceptions are a big improvement, IMO. I'll try to have a look soon. @twalthr, can you have a

[GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...

2016-04-20 Thread yjshen
GitHub user yjshen opened a pull request: https://github.com/apache/flink/pull/1916 [FLINK-3754][Table]Add a validation phase before construct RelNode using TableAPI This PR aims at adding an extra phase of **validation** for plans generated from Table API, matches the