Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE: fast-binary-sort adds TBase OrderedSerialization #1212

Closed
wants to merge 7 commits into from

Conversation

johnynek
Copy link
Collaborator

@johnynek johnynek commented Feb 26, 2015

This is on top of #1210 only merge after that has been merged.

Tasks:

  • This has disabled the scrooge-tbase comparators. We need to consider which route to go. One solution may be to provide both the macro based and the TBase approach for scrooge and let users opt in to one or the other.
  • Due to string comparison not matching binary comparison of Array[Byte] due to string compare effectively being unsigned, but Array[Byte]/ByteBuffer comparison being signed we need to be careful about the OrderedSerialization contract. We should do our best to find a high performance way to follow the laws.
  • Internal to Twitter there may be a more recent TBase comparator. We need to make sure we have the latest and greatest here.

@mcanlas
Copy link

mcanlas commented Aug 30, 2016

Stale PR. Should we close?

@johnynek
Copy link
Collaborator Author

johnynek commented Sep 23, 2017

yeah, I think no one I know of is really dying to use OrderedSerialization with scalding with thrift (other than scrooge).

@johnynek johnynek closed this Sep 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants