-
Notifications
You must be signed in to change notification settings - Fork 218
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
Hash code updates. #394
Open
PaulSandoz
wants to merge
3
commits into
Netflix:master
Choose a base branch
from
PaulSandoz:set-map-hash-code-tests
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Hash code updates. #394
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3715b14
to
a5fc9b9
Compare
dkoszewnik
requested changes
Feb 19, 2019
hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowBlobReader.java
Show resolved
Hide resolved
hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java
Outdated
Show resolved
Hide resolved
hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java
Outdated
Show resolved
Hide resolved
hollow/src/main/java/com/netflix/hollow/core/schema/HollowSchema.java
Outdated
Show resolved
Hide resolved
5e0f859
to
547b8b2
Compare
I pushed another commit that:
|
e45ac90
to
3885fd3
Compare
- Simplfy whether a set or map schema specifies a hash function for ordinal values of elements or keys - Fix issue checking for schema mistmatch, and test
This feature seems overly complex and has little value especially when the deprecated HollowObjectHashCodeFinder is removed. A declaration of @HollowHashCode(fields = {}) overrides the default hash key generation for a schema as performed by the object mapper.
3885fd3
to
9105940
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request contains a number of updates related to the hash code of sets and maps primarily to enable the removal of
HollowObjectHashCodeFinder
and related hash code functionality at least on the producer side and limited use on the client side (specifically to ensure efficient lookup of elements or keys when usingHollowSet
orHollowMap
until a solution is implemented that can reliably extract a hash key from an object).Support
@HollowHashKey
and hash key with empty fields for hollow set and map schema. This feature can override the application of a default hash key (seeHollowObjectMapper.useDefaultHashKeys
which is true by default) when it is required that ordinals be used as the hash code.Allow a consumer to filter out hash keys of set or map schema. The consumer needs to register an instance of
HollowObjectHashCodeFinder
whose methodgetTypesWithDefinedHashCodes
returns aSet
that contains the element or key type of the set or map schema. This is a breaking change for an edge case: prior to this change a set/map with a hash key would override that ofHollowObjectHashCodeFinder
. However, prior to this change it was not expected that a consumer implementgetTypesWithDefinedHashCodes
with anything but an empty set.Ensure a map or set whose element or key type is filtered (or not present) can still be processed. Any hash key is ignored since the field paths cannot be resolved.
When performing a checksum calculation for a set or map relax the schema equality check for the hash keys if the schema evolved from no hash key to with a hash key. This enables a transition from no hash key (the hash is defined externally using
HollowObjectHashCodeFinder
) to a hash key (the hash is defined by the hash key on the schema).Ensure a producer can transition (with restoration) from a state where
HollowObjectHashCodeFinder
was used (and where default hash keys were not enabled) to generate hash codes to whereHollowObjectHashCodeFinder
is not used (and where where default hash keys are enabled). If hash codes are the same in both cases then the checksum calculation performed in the integrity check stage should not fail.