diff --git a/fragment.go b/fragment.go index b0bf22e05..9734429ed 100644 --- a/fragment.go +++ b/fragment.go @@ -1330,19 +1330,20 @@ func (f *fragment) rangeEQ(tx Tx, bitDepth uint, predicate int64) (*Row, error) return nil, err } - // Filter to only positive/negative numbers. upredicate := absInt64(predicate) + if uint(bits.Len64(upredicate)) > bitDepth { + // Predicate is out of range. + return NewRow(), nil + } + + // Filter to only positive/negative numbers. + r, err := f.row(tx, bsiSignBit) + if err != nil { + return nil, err + } if predicate < 0 { - r, err := f.row(tx, bsiSignBit) - if err != nil { - return nil, err - } b = b.Intersect(r) // only negatives } else { - r, err := f.row(tx, bsiSignBit) - if err != nil { - return nil, err - } b = b.Difference(r) // only positives } @@ -1428,18 +1429,10 @@ func (f *fragment) rangeLT(tx Tx, bitDepth uint, predicate int64, allowEquality } } -// msb gives the 1-indexed position (counting from lsb) of the most -// significant bit. E.G. for 1 it would return 1, for 2 2, for 3 2, -// for 4 3, for 8 4, etc. -func msb(x uint64) uint { - lz := bits.LeadingZeros64(x) - return 64 - uint(lz) -} - // rangeLTUnsigned returns all bits LT/LTE the predicate without considering the sign bit. func (f *fragment) rangeLTUnsigned(tx Tx, filter *Row, bitDepth uint, predicate uint64, allowEquality bool) (*Row, error) { switch { - case msb(predicate) > bitDepth: + case uint(bits.Len64(predicate)) > bitDepth: fallthrough case predicate == (1< bitDepth: + // The predicate is bigger than the BSI width, so nothing can be bigger. + return NewRow(), nil case allowEquality: predicate-- + allowEquality = false + goto prep } // Compare intermediate bits. @@ -1636,9 +1635,9 @@ func (f *fragment) rangeBetweenUnsigned(tx Tx, filter *Row, bitDepth uint, predi } // Compare any upper bits which are equal. - firstDiff := int(msb(predicateMax^predicateMin)) - 1 + diffLen := bits.Len64(predicateMax ^ predicateMin) remaining := filter - for i := int(bitDepth - 1); i > firstDiff; i-- { + for i := int(bitDepth - 1); i >= diffLen; i-- { row, err := f.row(tx, uint64(bsiOffsetBit+i)) if err != nil { return nil, err @@ -1651,12 +1650,17 @@ func (f *fragment) rangeBetweenUnsigned(tx Tx, filter *Row, bitDepth uint, predi } } + // Clear the bits we just compared. + equalMask := (^uint64(0)) << diffLen + predicateMin &^= equalMask + predicateMax &^= equalMask + var err error - remaining, err = f.rangeGTUnsigned(tx, remaining, uint(firstDiff+1), predicateMin, true) + remaining, err = f.rangeGTUnsigned(tx, remaining, uint(diffLen), predicateMin, true) if err != nil { return nil, err } - remaining, err = f.rangeLTUnsigned(tx, remaining, uint(firstDiff+1), predicateMax, true) + remaining, err = f.rangeLTUnsigned(tx, remaining, uint(diffLen), predicateMax, true) if err != nil { return nil, err } diff --git a/fragment_internal_test.go b/fragment_internal_test.go index bf1a0116d..06672a861 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -710,6 +710,34 @@ func TestFragment_Range(t *testing.T) { } }) + t.Run("EQOversizeRegression", func(t *testing.T) { + f, idx := mustOpenFragment("i", "f", viewStandard, 0, "") + _ = idx + defer f.Clean(t) + + // Obtain transaction. + tx := &RoaringTx{fragment: f} + + // Set values. + if _, err := f.setValue(tx, 1000, 1, 0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(tx, 2000, 1, 1); err != nil { + t.Fatal(err) + } + + // Query for equality. + if b, err := f.rangeOp(tx, pql.EQ, 1, 3); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(b.Columns(), []uint64{}) { + t.Fatalf("unexpected columns: %+v", b.Columns()) + } + if b, err := f.rangeOp(tx, pql.EQ, 1, 4); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(b.Columns(), []uint64{}) { + t.Fatalf("unexpected columns: %+v", b.Columns()) + } + }) + t.Run("NEQ", func(t *testing.T) { f, idx := mustOpenFragment("i", "f", viewStandard, 0, "") _ = idx @@ -908,6 +936,27 @@ func TestFragment_Range(t *testing.T) { } }) + t.Run("GTOversizeRegression", func(t *testing.T) { + f, idx := mustOpenFragment("i", "f", viewStandard, 0, "") + _ = idx + defer f.Clean(t) + + // Obtain transaction. + tx := &RoaringTx{fragment: f} + + if _, err := f.setValue(tx, 1, 2, 0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(tx, 2, 2, 1); err != nil { + t.Fatal(err) + } + + if b, err := f.rangeGTUnsigned(tx, NewRow(1, 2), 2, 4, false); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(b.Columns(), []uint64{}) { + t.Fatalf("unepxected coulmns: %+v", b.Columns()) + } + }) + t.Run("BETWEEN", func(t *testing.T) { f, idx := mustOpenFragment("i", "f", viewStandard, 0, "") _ = idx @@ -960,6 +1009,27 @@ func TestFragment_Range(t *testing.T) { t.Fatalf("unexpected columns: %+v", b.Columns()) } }) + + t.Run("BetweenCommonBitsRegression", func(t *testing.T) { + f, idx := mustOpenFragment("i", "f", viewStandard, 0, "") + _ = idx + defer f.Clean(t) + + // Obtain transaction. + tx := &RoaringTx{fragment: f} + + if _, err := f.setValue(tx, 1, 64, 0xf0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(tx, 2, 64, 0xf1); err != nil { + t.Fatal(err) + } + + if b, err := f.rangeBetweenUnsigned(tx, NewRow(1, 2), 64, 0xf0, 0xf1); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(b.Columns(), []uint64{1, 2}) { + t.Fatalf("unepxected coulmns: %+v", b.Columns()) + } + }) } // benchmarkSetValues is a helper function to explore, very roughly, the cost