Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1465
---
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
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50973634
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,231 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50974438
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50977035
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50978510
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-175601474
Looks good 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 not have
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-175603356
I will merge it, once the build succeeded.
---
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 fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50721087
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,231 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50724626
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50724445
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r50722619
--- Diff:
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,423 @@
+/*
+ * Licensed
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-174595730
PR looks good. Just a few minor things to fix before it can be merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-174166083
No problem, @twalthr. Thanks for the update! Will review it in the next
days.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-174157061
@fhueske I have implemented your feedback and rebased it onto the current
master. Unfortunately, Git had problems with the new package structure. I had
to squash my
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49448809
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49449610
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49438915
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49454443
--- Diff:
flink-staging/flink-table/src/test/scala/org/apache/flink/api/table/typeinfo/RowComparatorTest.scala
---
@@ -0,0 +1,112 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49455376
--- Diff:
flink-staging/flink-table/src/test/scala/org/apache/flink/api/table/typeinfo/RowComparatorTest.scala
---
@@ -0,0 +1,112 @@
+/*
+ *
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170916212
Hi Timo, the PR looks good except for the `extractKeys` and
`getFlatComparator` methods. I think it is possible to support composite type
wrapping with the
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170918270
@fhueske Thanks for reviewing my code so intensively. Your suggestion
sounds very good. But again I have the problem of how to access the order of
the atomic types
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170921788
The `NullAwareComparator` knows about the order of its wrapped comparator.
So we can implement the `getFlatComparators()` method of `NullAwareComparator`,
call
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49440748
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49441097
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49449125
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49436274
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49436263
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49440308
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49441770
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49448186
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,422 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49450440
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,422 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49439105
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49442539
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49439808
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49441925
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r49442607
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullAwareComparator.scala
---
@@ -0,0 +1,186 @@
+/*
+ *
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170934568
Yes, I think so.
ATM, the Table API does not allow to access nested fields but only Row
fields as a whole. So there is no way to define different order for nested
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170930122
Sorry, I mixed up atomic and basic types. I forgot that a flat comparators
list could also contain GenericTypes etc.
Can we assume that the orders of the flat
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-170584470
@fhueske I updated the PR again. I implemented your suggestion of
NullAwareComparators wrapping the normal comparators. I also added additional
test cases. What do you
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-169275924
Thanks for your feedback @fhueske. I will rework this PR during this week.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48261874
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowSerializer.scala
---
@@ -40,15 +46,32 @@ class RowSerializer(val
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48285450
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48282283
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48288721
--- Diff:
flink-staging/flink-table/src/test/scala/org/apache/flink/api/table/typeinfo/RowSerializerTest.scala
---
@@ -0,0 +1,125 @@
+/*
+ *
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48286873
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48287946
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48288039
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/BasicTypeComparator.java
---
@@ -42,6 +42,10 @@ protected
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48282226
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48285315
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,463 @@
+/*
+ * Licensed to
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-165819420
I now use lazy vals for that.
---
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
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-165813593
I implemented your feedback. But I don't know if we should use `Option` for
the `deserializedKeyFields`, because we then need to add checks for each
serialized
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48025862
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r48026706
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
GitHub user twalthr opened a pull request:
https://github.com/apache/flink/pull/1465
[FLINK-3140] NULL value data layout in Row Serializer/Comparator
This PR implements FLINK-3140 and enables full NULL field support for
grouping and sorting rows.
You can merge this pull request
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47903321
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47902384
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullMaskUtils.scala
---
@@ -0,0 +1,92 @@
+/*
+ *
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-165474222
@aljoscha is absolutely right about the performance aspect. However, the
key difference is that he was referring in his comment to the implementation of
the
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47907286
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47903103
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullMaskUtils.scala
---
@@ -0,0 +1,92 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47911141
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47913064
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47913887
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47911612
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47903615
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1465#issuecomment-165462700
Thanks for your feedback @tillrohrmann.
Many parts could be expressed in more Scala style, but for performance
reasons and to prevent bugs in this crucial component
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47903444
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ *
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47912714
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/RowComparator.scala
---
@@ -0,0 +1,468 @@
+/*
+ * Licensed to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1465#discussion_r47918892
--- Diff:
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/typeinfo/NullMaskUtils.scala
---
@@ -0,0 +1,92 @@
+/*
+ *
68 matches
Mail list logo
