Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/2025#issuecomment-221762766
Hi @fhueske , I've updated the `expectedType` fix and reverted changes in
`DataSetUnion` and `DataSetUnionRule` to make the implementation clear and
simple, what do you
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64547780
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/nodes/dataset/DataSetUnion.scala
---
@@ -69,16 +73,23 @@ class
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64546723
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/nodes/dataset/DataSetUnion.scala
---
@@ -69,16 +73,23 @@ class
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/2025#issuecomment-221508839
@fhueske would you please take another look? Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64383299
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/nodes/dataset/DataSetUnion.scala
---
@@ -69,16 +73,23 @@ class
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64382835
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/nodes/dataset/DataSetUnion.scala
---
@@ -69,16 +73,23 @@ class
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64377174
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/runtime/aggregate/AggregateMapFunction.scala
---
@@ -44,7 +44,7 @@ class
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2025#discussion_r64371728
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/runtime/aggregate/AggregateMapFunction.scala
---
@@ -44,7 +44,7 @@ class
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/2025#issuecomment-221225446
Hi @fhueske , thanks for the review! The changes are:
- Remove the whole `java/batch/table/UnionITCase.java`
- Remove `testUnionWithFilter`, `testUnionWithJoin
GitHub user yjshen opened a pull request:
https://github.com/apache/flink/pull/2025
[FLINK-3941][TableAPI]Add support for UNION (with duplicate elimination)
This PR aims at adding `UNION` support in TableAPI and SQL by:
- Extending Table API with a new `union()` method
Github user yjshen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2014#discussion_r64160758
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/rules/dataSet/DataSetAggregateRule.scala
---
@@ -33,20 +33,32 @@ class
GitHub user yjshen opened a pull request:
https://github.com/apache/flink/pull/2015
[FLINK-3632][TableAPI]Clean up Table API exceptions
As suggested by @fhueske in
https://github.com/apache/flink/pull/1958#discussion_r62680699, we should use
`TableException` in the following cases
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/2004#issuecomment-220068286
@fhueske @twalthr could you please take a look at this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
GitHub user yjshen opened a pull request:
https://github.com/apache/flink/pull/2004
[Flink-3926][TypeSystem]Incorrect implementation of getFieldIndex in
TupleTypeInfo
The origin code assumes filed name as `f{i}` which is not always true.
Finding a field `a` would result
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
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 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 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 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
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 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 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 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 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 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 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 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 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 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 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 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 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 user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1961#issuecomment-216877262
Hi @fhueske , thanks for the explanation. If I get it correctly, the
current `toSink` API is a general one to allow write table content to a large
variety of `Sinks
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1961#issuecomment-216575391
Hi @fhueske , I've read through this PR and find a little wired of the
current API design.
Please correct me if I take something wrong: Since we are output
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] flink-connector
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 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
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
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
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 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 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
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 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 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 er
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 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 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
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 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 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(
* tab.filter
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
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1880#issuecomment-209990696
Are there other systems support `not nullCheck` mode?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1880#issuecomment-209954946
I happened to have context here for `primitiveDefaultValue`, I think this
was originated from Apache Spark's counterpart for create default values for an
expression. Let
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1880#issuecomment-209836875
Hi @twalthr, why we need a null literal? Please forgive me if this is a
silly question...
---
If your project is set up for it, you can reply to this email and have
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1870#issuecomment-208829993
I am glad I was able to help. Thanks for the reviewing work @fhueske!
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1870#issuecomment-208663670
@fhueske, thanks :). Please let me know your thoughts when you give another
pass.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user yjshen commented on the pull request:
https://github.com/apache/flink/pull/1870#issuecomment-208661457
The build failure seems unrelated to this PR:
```
[INFO] Compiling 43 source files to
/home/travis/build/apache/flink/flink-libraries/flink-ml/target/classes
GitHub user yjshen opened a pull request:
https://github.com/apache/flink/pull/1870
[FLINK-3736][TableAPI]Move toRexNode & toAggCall logic into each
expression's implementation
Since we have a one-to-one mapping from Flink `Expression` to Calcite
`RexNode`, I think it's a
59 matches
Mail list logo
