GitHub user mxm opened a pull request:
https://github.com/apache/flink/pull/466
[FLINK-1622][java-api][scala-api] add a partial GroupReduce operator
The partial GroupReduce operator acts like a regular GroupReduce
operator but does not perform a full group reduce. Instead, it per
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26064533
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/operators/base/GroupReducePartialOperatorBase.java
---
@@ -0,0 +1,150 @@
+/*
+ * Licen
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26105008
--- Diff:
flink-compiler/src/main/java/org/apache/flink/compiler/operators/GroupReducePartialProperties.java
---
@@ -0,0 +1,112 @@
+/*
+ * Licensed t
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26105136
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SortedGrouping.java
---
@@ -156,6 +156,23 @@ public SortedGrouping(DataSet set, Keys k
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26105155
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/UnsortedGrouping.java
---
@@ -159,7 +159,23 @@ public UnsortedGrouping(DataSet set, Ke
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26105226
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/GroupedDataSet.scala ---
@@ -355,6 +355,63 @@ class GroupedDataSet[T: ClassTag](
}
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78016698
I like the implementation, except for my comments on groupReducePartial()
on grouped DataSets. Also, the tests seem a bit shady because of all the
grouping and regular re
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26120661
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SortedGrouping.java
---
@@ -156,6 +156,23 @@ public SortedGrouping(DataSet set, Keys keys,
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26130403
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/GroupedDataSet.scala ---
@@ -355,6 +355,63 @@ class GroupedDataSet[T: ClassTag](
}
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26130234
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/operators/base/GroupReducePartialOperatorBase.java
---
@@ -0,0 +1,150 @@
+/*
+ * Licensed t
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26130398
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/UnsortedGrouping.java
---
@@ -159,7 +159,23 @@ public UnsortedGrouping(DataSet set, Keys ke
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78076027
@aljoscha Thanks for the comments. I agree, the tests are a bit shady
because they test the operator by first performing a partial, then a full
reduce. Using a custom partitio
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78070448
I assume this will be new operator but I do not see updates on the
documentation files.
---
If your project is set up for it, you can reply to this email and have your
r
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78084575
Yes, I've got a couple of comments as well.
First of all, as @mxm said, I would propose to call this operator
``combine`` because it is a generalized combiner (out
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78093403
@aljoscha @fhueske For a general combine, the operator can be used without
grouping. When we want to combine elements before performing a proper
groupReduce with a groupBy, we
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26139203
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/GroupReducePartialOperator.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apach
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/466#discussion_r26135427
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/GroupReducePartialOperator.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the A
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78091524
I have to correct myself. A combiner should of course be called on groups
of records. Therefore, calling it on a Grouping makes absolute sense. However,
the semantics of t
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-78101926
Sorry, I completely blanked, of course, You still need the grouping, only
the shuffle step you don't need.
So, I suggest only better tests, using a combination of
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/466#issuecomment-79096322
@aljoscha @fhueske @hsaputra Thanks for the feedback. Some people suggested
that the name is confusing and that my pull request involved too much code
duplication. I propose t
20 matches
Mail list logo
