Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-199798994
Closed this PR in favor of PR #1822
---
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 user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1553
---
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/1553#issuecomment-198364412
Hi @ramkrish86, I thought about this PR and came to the conclusion that we
should not continue. The optimizer's design does not allow to modify operators
in or inject op
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-193665240
@fhueske
Everthing is a learning. good that I got to know some flows out of this
issue. Ya am interested to take up some other JIRA in the meantime.
---
If your
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-193658558
Hi @ramkrish86, I totally understand that you are disappointed. I'm very
sorry to raise these concerns this late after you put a lot of effort into this
PR. I should hav
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-193595361
@fhueske
I was looking into the comments and the refactoring I can avoid by creating
a new patch altogether. But saying the last comment I think I can hold this o
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-193272515
Hi Ram,
I just realized that the approach taken here might not work. We are
modifying the plan while it is enumerated. There might be cases, where this
leads to com
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55208348
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55208210
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55205009
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55205422
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55205324
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55205361
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55205091
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r55204942
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -126,12 +117,66 @@ public SingleInp
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-192131350
New PR submitted @fhueske . Thanks for helping me thro this code review. It
is was more of a beginner and there is a lot to learn from my side.
---
If your project
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-191752181
Sorry, I forgot a `groupBy()` in my example.
It should be
```
DataSet> data = ...
DataSet> pData = data.partitionByHash(0);
pData.map(new SomeMapF
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-191748107
@fhueske
So for doing the above example where the partioned input goes both to the
map and reducer as input
should this class AllGroupWithPartialPreGroupProp
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-190124885
I do not have a concrete use case in mind, but it is certainly possible to
implement such a job in the DataSet API. Hence, it should be correctly
translated.
You ca
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r54386465
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user ramkrish86 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r54329786
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverSt
Github user ramkrish86 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r54329750
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfG
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-189599549
@fhueske
Could you give some examples for the above use case where the partition is
an input to more than one function?
---
If your project is set up for it,
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53998405
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfGrou
Github user ramkrish86 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53978205
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfG
Github user ramkrish86 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53659684
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfG
Github user ramkrish86 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53659650
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverSt
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53153223
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/ReduceProperties.java
---
@@ -59,37 +65,69 @@ public DriverStrategy getStrategy(
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53153386
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfGrou
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152587
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152922
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152668
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53153193
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/ReduceProperties.java
---
@@ -59,37 +65,69 @@ public DriverStrategy getStrategy(
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152430
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152240
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152054
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -110,28 +135,49 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152023
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -110,28 +135,49 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53151692
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53152093
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -110,28 +135,49 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53151777
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53151545
--- Diff:
flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/wordcount/WordCount.java
---
@@ -66,7 +66,7 @@ public static void
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53151628
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -110,28 +135,49 @@ public SingleInp
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53151578
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -87,19 +92,39 @@ public DriverStrat
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-185170325
Hi @ramkrish86, thanks for the update.
In addition to my comments inline we also need to extend the `ReduceITCase`.
Also we must take care of the case where t
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r53153472
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/javaApiOperators/GroupReduceITCase.java
---
@@ -434,6 +431,95 @@ public void
testCorrectnessOfGrou
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-185025591
A new push request has been submitted. JYFI @fhueske .
---
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 fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-183230276
@ramkrish86, no worries :-)
I guess the issue description lacked a bit of detail. Flink's optimizer
checks, if the partitioning produced by the explicit partitioning
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-183190815
@fhueske
I got the problem that I was making. My bad. I was not applying the
partition function on the Key ie. the String part returned from the flat map
and hen
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-183014873
If a `Partitioner` is implemented such that is does not partition based on
the key attribute, it cannot be used for a Reduce or GroupReduce transformation
anyways. Also
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-182969700
Might be a stupid question, but what if the partitioner depends on the
number of elements. E.g. if you use `partitionCustom` with `Partitioner` which
counts interna
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-177997324
The `GroupReduceWithCombineProperties.instanciate()` method checks the
shipping strategy of the input channel. In case of the `WordCount` example
*without* explicit hash
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-177957906
I went through the code. In both cases of WordCount program with and
without explicit partition
`[Map] --hash-partition--> [Reduce]`
`[Map] --partition--> [Par
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-177080713
Thank you very much for the feedback. Let me try to understand this thing
better and update the PR sooner. I will reach out here in case of any questions
or doubts th
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-176754489
You identified the right classes and methods for the fix, but the place
within the method is not correct. Let me explain the issue.
In the common case as for exa
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r51259080
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/ReduceProperties.java
---
@@ -66,30 +70,62 @@ public SingleInputPlanNode instant
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1553#discussion_r51259067
--- Diff:
flink-optimizer/src/main/java/org/apache/flink/optimizer/operators/GroupReduceWithCombineProperties.java
---
@@ -102,36 +107,72 @@ public SingleInp
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-176059338
Thanks for the PR!
I'll have a look at it and give feedback hopefully today or tomorrow.
---
If your project is set up for it, you can reply to this email and have
Github user ramkrish86 commented on the pull request:
https://github.com/apache/flink/pull/1553#issuecomment-175730064
Also ensured that the related test cases passes and also the Wordcount
program output with and without partition remains the same.
---
If your project is set up for
GitHub user ramkrish86 opened a pull request:
https://github.com/apache/flink/pull/1553
FLINK-3179 Combiner is not injected if Reduce or GroupReduce input is
explicitly partitioned (Ram)
Followed the guidance given in the description in order to fix this. Is the
approach correct he
59 matches
Mail list logo
