Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1983#issuecomment-221873359
Thanks, Greg, this looks good all in all!
A few thinks we should do before merging this, in my opinion:
- I think all classes should be annotated with `@Internal`, because they
should not be used directly by users.
- The `NullValueComparator` can be simplified to not really operate on
the values at all. All NullValues are always the same. They are all equal, hash
to a constant value, etc.
- We were trying to get rid of the `Record` type - it was part of a very
old legacy API. It is still in there because some people ended up using it, but
I would like to add as little dependency to it as possible. Hence I'd suggest
to drop the `RecordSerializer`.
---
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 enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
