Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-138594491
Thanks @r-pogalz and @jkovacs!
---
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 h
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1052
---
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 enab
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-138568345
Will merge this 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 project does not have this fe
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-138491135
I did not go through all details, but from what I read it was good. Go
ahead from my side...
---
If your project is set up for it, you can reply to this email and h
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-138337040
Looks good to me. Will merge this tomorrow, if nobody objects.
Thanks @r-pogalz and @jkovacs!
---
If your project is set up for it, you can reply to this email and h
Github user r-pogalz commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-138221929
The warnings fix commit is ready for review.
---
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 pro
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-137781257
Yeah, adding warnings fix to this PR is good.
---
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 r-pogalz commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-137780075
Thanks for you comment @StephanEwen, good to know that. I tried to minimize
the occurrences of warnings where possible by fixing or suppressing it for this
issue. Shoul
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-137757977
I just saw that the OuterJoin runtime classes/tests cause a lot of warnings
when compiling. Manyly raw types, unchecked casts.
Would be good to fix those. In
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-137691399
Looks good to merge. :+1:
---
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 hav
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-137483619
Hi @r-pogalz, @jkovacs,
thanks for the update. It's good to merge, IMO.
Either you resolve my minor comments or I can do it before merging.
Anybody
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38654830
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/LeftOuterJoinTaskExternalITTest.java
---
@@ -0,0 +1,41 @@
+/*
+ * Licensed t
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38654895
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/RightOuterJoinTaskExternalITTest.java
---
@@ -0,0 +1,41 @@
+/*
+ * Licensed
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38654400
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/FullOuterJoinTaskExternalITTest.java
---
@@ -0,0 +1,41 @@
+/*
+ * Licensed t
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38654270
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,452 @@
+/*
+ * Licensed to the
Github user r-pogalz commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-136056199
Hi @tillrohrmann, @fhueske and @chiwanpark,
thanks for your reviews and comments. I went through the classes and
changed the formatting. Moreover, I refactored the
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38002104
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed to the
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-135084756
Thanks @r-pogalz and @jkovacs for contribution!
I found some code style issues.
* Please add spaces around operator and after comma.
* Please don't add
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38001503
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38001220
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed to the
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r38000668
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-135079785
Thanks @r-pogalz & @jkovacs for this excellent PR!
I didn't see anything critical.
IMO, it's good to merge once you addressed @tillrohrmann's and
@chiwanpark's c
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r3704
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed to the
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37999088
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Li
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37999081
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Li
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37999044
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Li
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37998836
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Li
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37998816
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Li
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37998051
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/SortMergeJoinDescriptor.java
---
@@ -99,7 +99,7 @@ public DualInputPlanNode i
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37997793
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed to the
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37994603
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ * Licen
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37980877
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the A
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37979571
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the A
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37979451
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the A
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134902134
@r-pogalz I would implement API and optimizer together in one issue. IMO it
is easier to start from the API and then work your way through API to optimizer
and finally u
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134901571
Hi @r-pogalz, usually you only have one PR for a ticket. Thus, it would be
good to create two new sub-tasks for the API and the optimizer integration.
---
If your
Github user r-pogalz commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134898862
Hi @tillrohrmann, thanks for your review and comments. I will work through
it soon.
Regarding the ticket FLINK-2106, @fhueske suggested us to create multiple
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134567455
Really good work @r-pogalz. I had only some minor comments concerning style
and test cases.
I like your approach to split the implementation of FLINK-687 i
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846113
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846982
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/testutils/BinaryOperatorTestBase.java
---
@@ -0,0 +1,418 @@
+/*
+ * Lic
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846918
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/testutils/BinaryOperatorTestBase.java
---
@@ -0,0 +1,418 @@
+/*
+ * Lic
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846359
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846291
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846212
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846160
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37845999
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846048
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37846793
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskTest.java
---
@@ -0,0 +1,501 @@
+/*
+ * Licensed t
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37845794
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/AbstractOuterJoinTaskExternalITCase.java
---
@@ -0,0 +1,121 @@
+/*
+ *
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134521057
Thanks for your contribution @r-pogalz. Could you please add some more
details about the implementation to the PR description.
---
If your project is set up for it
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37844233
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37844055
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37844335
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/FullOuterJoinDriver.java
---
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the A
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37844281
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/FullOuterJoinDriver.java
---
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the A
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37843780
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1052#discussion_r37843359
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/AbstractOuterJoinDriver.java
---
@@ -0,0 +1,166 @@
+/*
+ * Licensed to
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1052#issuecomment-134457512
Hi, Thanks for your contribution. I'll review this and leave comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
57 matches
Mail list logo