diff --git a/fragment.go b/fragment.go index 4121a4424..a8e5ad4fe 100644 --- a/fragment.go +++ b/fragment.go @@ -1230,12 +1230,18 @@ func (f *fragment) rangeEQ(bitDepth uint, predicate int64) (*Row, error) { // Start with set of columns with values set. b := f.row(bsiExistsBit) - // 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 := f.row(bsiSignBit) if predicate < 0 { - b = b.Intersect(f.row(bsiSignBit)) // only negatives + b = b.Intersect(r) // only negatives } else { - b = b.Difference(f.row(bsiSignBit)) // only positives + b = b.Difference(r) // only positives } // Filter any bits that don't match the current bit value. @@ -1308,18 +1314,10 @@ func (f *fragment) rangeLT(bitDepth uint, predicate int64, allowEquality bool) ( } } -// 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(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. @@ -1479,9 +1483,9 @@ func (f *fragment) rangeBetweenUnsigned(filter *Row, bitDepth uint, predicateMin } // 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 := f.row(uint64(bsiOffsetBit + i)) switch (predicateMin >> uint(i)) & 1 { case 1: @@ -1491,12 +1495,17 @@ func (f *fragment) rangeBetweenUnsigned(filter *Row, bitDepth uint, predicateMin } } + // Clear the bits we just compared. + equalMask := (^uint64(0)) << uint(diffLen) + predicateMin &^= equalMask + predicateMax &^= equalMask + var err error - remaining, err = f.rangeGTUnsigned(remaining, uint(firstDiff+1), predicateMin, true) + remaining, err = f.rangeGTUnsigned(remaining, uint(diffLen), predicateMin, true) if err != nil { return nil, err } - remaining, err = f.rangeLTUnsigned(remaining, uint(firstDiff+1), predicateMax, true) + remaining, err = f.rangeLTUnsigned(remaining, uint(diffLen), predicateMax, true) if err != nil { return nil, err } diff --git a/fragment_internal_test.go b/fragment_internal_test.go index fa1ffbeec..64e3cb326 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -538,6 +538,30 @@ func TestFragment_Range(t *testing.T) { } }) + t.Run("EQOversizeRegression", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Clean(t) + + // Set values. + if _, err := f.setValue(1000, 1, 0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(2000, 1, 1); err != nil { + t.Fatal(err) + } + + // Query for equality. + if b, err := f.rangeOp(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(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 := mustOpenFragment("i", "f", viewStandard, 0, "") defer f.Clean(t) @@ -706,6 +730,23 @@ func TestFragment_Range(t *testing.T) { } }) + t.Run("GTOversizeRegression", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Clean(t) + + if _, err := f.setValue(1, 2, 0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(2, 2, 1); err != nil { + t.Fatal(err) + } + + if b, err := f.rangeGTUnsigned(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 := mustOpenFragment("i", "f", viewStandard, 0, "") defer f.Clean(t) @@ -753,6 +794,23 @@ func TestFragment_Range(t *testing.T) { t.Fatalf("unexpected columns: %+v", b.Columns()) } }) + + t.Run("BetweenCommonBitsRegression", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Clean(t) + + if _, err := f.setValue(1, 64, 0xf0); err != nil { + t.Fatal(err) + } else if _, err := f.setValue(2, 64, 0xf1); err != nil { + t.Fatal(err) + } + + if b, err := f.rangeBetweenUnsigned(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