Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1043
---
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 StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-135512163
Will merge this...
---
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 user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134639235
Looks good
+1 to merge
---
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 fhueske commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134665239
Thanks! Good to merge, IMO.
---
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 zentol commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134663435
@fhueske Added the test you requested.
---
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 doe
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134618042
@StephanEwen I've reimplemented hashCode() and compare() accordingly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134603166
I think you currently pay quite a performance price for implementing the
compare methods in such a ways that they delegate to the `ByteComparator` and
`FloatComparat
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134565718
Fair enough, I'll add another test.
---
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 n
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134563069
This change touches the DataSet API, so there should be a test there as
well, IMO.
Also this feature would not be tested until the Python API is adapted.
---
If yo
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134536795
Looks good.
Maybe add one more test that executes one of the comparators, for example
in GroupReduceITCase?
---
If your project is set up for it, you can reply to
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134538956
The Python API will heavily rely on this, so actual use-cases should be
sufficiently covered there.
---
If your project is set up for it, you can reply to this email and
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1043#discussion_r37844225
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/operator/GroupingTest.java
---
@@ -127,6 +128,15 @@ public void testGroupByKeyFields5() {
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1043#discussion_r37842526
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/operator/GroupingTest.java
---
@@ -127,6 +128,15 @@ public void testGroupByKeyFields5() {
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134397104
Looks 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 project does not have this featur
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134345186
I've added a test case to make sure a primitive array is accepted as a key.
is that what you had in mind @tillrohrmann ?
---
If your project is set up for it, you can re
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1043#issuecomment-134167882
Good work @zentol. Could we also add a test case where we actually use a
primitive array as a key for some operation?
Other than that, LGTM +1.
---
If you
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1043#discussion_r37729602
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/array/BooleanPrimitiveArrayComparator.java
---
@@ -0,0 +1,56 @@
+/*
+
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1043#discussion_r37729559
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/array/BooleanPrimitiveArrayComparator.java
---
@@ -0,0 +1,56 @@
+/*
+ *
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1043#discussion_r37729380
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/array/BooleanPrimitiveArrayComparator.java
---
@@ -0,0 +1,56 @@
+/*
+
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/1043
[FLINK-2565] Support primitive Arrays as keys
Adds a comparator and test for every primitive array type.
Modifies the CustomType2 class in GroupingTest to retain a field with an
unsupported type.
20 matches
Mail list logo
