-
-
Notifications
You must be signed in to change notification settings - Fork 672
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
merge FastFieldCodecReader wit FastFieldDataAccess #1485
Conversation
PSeitz
commented
Aug 26, 2022
•
edited
Loading
edited
- num_vals to FastFieldCodecReader
- split open_from_bytes to own trait
- rename get_u64 to get_val
- merge FastFieldCodecReader and FastFieldDataAccess traits
Codecov Report
@@ Coverage Diff @@
## main #1485 +/- ##
==========================================
- Coverage 94.09% 94.07% -0.02%
==========================================
Files 239 239
Lines 44833 44863 +30
==========================================
+ Hits 42184 42207 +23
- Misses 2649 2656 +7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
9bd61c1
to
291232b
Compare
} | ||
|
||
impl FastFieldCodecReader for BitpackedReader { | ||
impl FastFieldCodecDeserializer for BitpackedReader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd put that in the XXXXSerializer
s and rename the trait XXXXCodec
, and add an associated trait over the reader.
The XXXXSerializer and the XXXXDeserializer are not really independent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think don't fully understand. I split the Deserializer from the FastFieldDataAccess
, because e.g. in the merge case we provide access without deserializing.