Repository navigation
Conversation
DeserializeFilterInput() compared the whole selector byte (0x20-0x2f) instead of its low 4 bits when decoding a SelectKeySegmentRange, so every serialized range was read back as segments 0 to 6. SerializeFilterInput() stores the range index in the low 4 bits, as the SelectKeySegment case already decodes. With a range other than 0 to 6, RangeMayMatch() compared query bounds and stored min/max taken from different key segments, and GetTableFilterForRangeQuery() could skip SST files containing keys in the range. Mask the byte to the low 4 bits before decoding, and add a test that runs a range query for every key segment range with a compact encoding.
|
Hi @Marti2203! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
DeserializeFilterInput()indb/experimental.ccread back everyserialized
SelectKeySegmentRangeasSelectKeySegmentRange(0, 6). This PRdecodes the range from the low 4 bits of the selector byte, as
SerializeFilterInput()encodes it and as theSelectKeySegmentcase justabove already does.
Root cause
SerializeFilterInput()writes a key segment range as one byte,(2 << 4) | index, whereindexis the range's position in the table in thecomment (0 = segments 0-1, 1 = 0-2, 2 = 1-2, ..., 15 = 0-6). Case 2 of
DeserializeFilterInput()compared the whole byteb(0x20 to 0x2f) against6, 10 and 15 instead of
b & 0xf. Sinceb >= 0x20, it always took the lastbranch and returned
SelectKeySegmentRange{0, 6}. Only a filter configuredwith segments 0-6 round-tripped, and only by chance.
The fix masks
bwithBitwiseAnd(b, 0xf)before the table lookup. I checkedall 16 encodable ranges against the encoder:
start = (to - 1) * to / 2givesindices 0 for
to= 1, 1-2 forto= 2, 3-5 forto= 3, 6-9 forto= 4,10-14 for
to= 5 and 15 for (0, 6). The existing decode branches invert thattable once they get the low nibble.
Impact
The bug affects the experimental SST query filter API (
SstQueryFilterConfigs,MakeSharedBytewiseMinMaxSQFC/MakeSharedReverseBytewiseMinMaxSQFC) when itis configured with any
SelectKeySegmentRangeother than (0, 6). The tableproperties collector computes the min/max over the configured segments. But
RangeMayMatch()extracts the query bounds over segments 0-6, so it comparesvalues from different parts of the key.
GetTableFilterForRangeQuery()canthen report that a file cannot match when it holds keys in the range. The
iterator skips that file, and the range query silently returns too few keys.
No error is reported.
Example: one-byte segments, filter on
SelectKeySegmentRange(1, 2), and an SSTwith keys
a1Zanda2Y. The stored min/max over segments 1-2 are1Zand2Y. A range query["a2Y", "a2Z")with the table filter returns nothing, butthe same query without the filter returns
a2Y.Files already written are not affected: the serialized selector byte was
correct, only the reader was wrong. So after this fix, existing filters are
read correctly.
Test plan
New test
DBBloomFilterTest.SstQueryFilterKeySegmentRangeindb/db_bloom_filter_test.cc. For each of the 16 key segment ranges with acompact encoding, it builds a DB with seven one-byte segments, flushes the keys
0000000and1111111, and checks two things:the key and seeks the file
Before the fix, the test fails for the 15 ranges other than (0, 6): the
matching key is missing and the file is never seeked. After the fix, it
passes. The existing
DBBloomFilterTest.SstQueryFilterandDBBloomFilterTest.FixedWidthSegmentsstill pass.This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually.