Boolean bitpacking #1884
Boolean bitpacking #1884
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1884 +/- ##
==========================================
- Coverage 89.73% 89.71% -0.02%
==========================================
Files 869 869
Lines 31126 31131 +5
==========================================
Hits 27930 27930
- Misses 3196 3201 +5
☔ View full report in Codecov by Sentry. |
benjaminwinger
force-pushed
the
bool-bitpacking-storage
branch
3 times, most recently
from
b933b5e
to
d1d5317
Compare
| @@ -52,7 +52,20 @@ struct PageElementCursor { | |||
| }; | |||
|
|
|||
| struct PageUtils { | |||
| static uint32_t getNumElementsInAPage(uint32_t elementSize, bool hasNull); | |||
| static constexpr uint32_t getNumElementsInAPage(uint32_t elementSize, bool hasNull) { | |||
There was a problem hiding this comment.
Why move from cpp to header file? If you want to keep it inside the header file, mark this function as inline.
There was a problem hiding this comment.
I'd moved it because so it can be constexpr, and constexpr (according to cppreference) implies inline (for cases where it's evaluated at runtime presumably).
Frankly, the static_assert I'd used it for is probably not necessary, but I thought it at least made more sense to make it a single static assert than multiple runtime asserts.
There was a problem hiding this comment.
Got it. Yeah, this makes sense.
benjaminwinger
force-pushed
the
bool-bitpacking-storage
branch
from
d1d5317
to
694395a
Compare
Implements boolean bitpacking for on-disk booleans and unpacks them when reading into ValueVectors.
Requires #1862.
I also optimized
ColumnChunk::templateCopyArrowArray<bool>to use copyNullMask instead of setting the bits individually. Arrow's null bits are flipped in comparison to kuzu's, so I modified the copy function to allow the result to be inverted.