diff --git a/executor.go b/executor.go index 538160cfd..3aa677c44 100644 --- a/executor.go +++ b/executor.go @@ -4845,6 +4845,13 @@ func getCondIntSlice(f *Field, cond *pql.Condition) ([]int64, error) { ret[i] = s } + // In the case where one (or both) of the predicates is on the + // opposite edge, return early to avoid the increment/decrement + // logic below and prevent an overflow. + if ret[0] == math.MaxInt64 || ret[1] == math.MinInt64 { + return ret, nil + } + switch cond.Op { case pql.BTWN_LT_LTE: // a < x <= b ret[0]++ diff --git a/executor_test.go b/executor_test.go index c434eaa75..e29dd6b99 100644 --- a/executor_test.go +++ b/executor_test.go @@ -2559,6 +2559,93 @@ func TestExecutor_Execute_Row_BSIGroup(t *testing.T) { }) } +// Ensure a Row(bsiGroup) query can be executed (edge cases). +func TestExecutor_Execute_Row_BSIGroupEdge(t *testing.T) { + c := test.MustRunCluster(t, 1) + defer c.Close() + hldr := test.Holder{Holder: c[0].Server.Holder()} + + idx, err := hldr.CreateIndex("i", pilosa.IndexOptions{}) + if err != nil { + t.Fatal(err) + } + + t.Run("LT", func(t *testing.T) { + if _, err := idx.CreateField("f1", pilosa.OptFieldTypeInt(-2000, 2000)); err != nil { + t.Fatal(err) + } + + // Set a value at the edge of bitDepth (i.e. 2^n-1; here, n=3). + // It must also be the max value in the field; in other words, + // set the value to bsiGroup.bitDepthMax(). + if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: ` + Set(100, f1=7) + `}); err != nil { + t.Fatal(err) + } + + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(f1 < 10)`}); err != nil { + t.Fatal(err) + } else if got, exp := result.Results[0].(*pilosa.Row).Columns(), []uint64{100}; !reflect.DeepEqual(got, exp) { + t.Fatalf("unexpected result: got=%v, exp=%v", got, exp) + } + }) + + t.Run("GT", func(t *testing.T) { + if _, err := idx.CreateField("f2", pilosa.OptFieldTypeInt(-2000, 2000)); err != nil { + t.Fatal(err) + } + + // Set a value at the negative edge of bitDepth (i.e. -(2^n-1); here, n=3). + // It must also be the min value in the field; in other words, + // set the value to bsiGroup.bitDepthMin(). + if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: ` + Set(200, f2=-7) + `}); err != nil { + t.Fatal(err) + } + + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(f2 > -10)`}); err != nil { + t.Fatal(err) + } else if got, exp := result.Results[0].(*pilosa.Row).Columns(), []uint64{200}; !reflect.DeepEqual(got, exp) { + t.Fatalf("unexpected result: got=%v, exp=%v", got, exp) + } + }) + + t.Run("BTWN_LT_LT", func(t *testing.T) { + if _, err := idx.CreateField("f3", pilosa.OptFieldTypeInt(-2000, 2000)); err != nil { + t.Fatal(err) + } + + // Set a value anywhere in range. + if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: ` + Set(300, f3=10) + `}); err != nil { + t.Fatal(err) + } + + // Query INT_MAX < x < INT_MIN. Because that's an invalid range, we should + // get back an empty result set. + tests := []struct { + predA int64 + predB int64 + }{ + {math.MaxInt64, math.MinInt64}, + {math.MaxInt64, 1000}, + {-1000, math.MinInt64}, + } + + for i, test := range tests { + pql := fmt.Sprintf("Row(%d < f3 < %d)", test.predA, test.predB) + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: pql}); err != nil { + t.Fatal(err) + } else if got, exp := result.Results[0].(*pilosa.Row).Columns(), []uint64{}; !reflect.DeepEqual(got, exp) { + t.Fatalf("test %d unexpected result: got=%v, exp=%v", i, got, exp) + } + } + }) +} + // Ensure a Range(bsiGroup) query can be executed. (Deprecated) func TestExecutor_Execute_Range_BSIGroup_Deprecated(t *testing.T) { c := test.MustRunCluster(t, 1) diff --git a/field.go b/field.go index 88ab0c128..d38aa2ffb 100644 --- a/field.go +++ b/field.go @@ -2189,11 +2189,6 @@ type bsiGroup struct { // ex: Field.Min = 0, Field.Max = 1023 // baseValue(LT, 2000) returns 1023, which will perform "LT 1023" and effectively // exclude any columns with value = 1023. -// Note that in this case (because the range uses the full BitDepth 0 to 1023), -// we can't simply return 1024. -// In order to make this work, we effectively need to change the operator to LTE. -// Executor.executeBSIGroupRangeShard() takes this into account and returns -// `frag.FieldNotNull(bsig.BitDepth())` in such instances. func (b *bsiGroup) baseValue(op pql.Token, value int64) (baseValue int64, outOfRange bool) { min, max := b.bitDepthMin(), b.bitDepthMax() @@ -2202,6 +2197,10 @@ func (b *bsiGroup) baseValue(op pql.Token, value int64) (baseValue int64, outOfR return baseValue, true } else if value < min { baseValue = int64(min - b.Base) + // Address edge case noted in comments above. + if op == pql.GT { + baseValue-- + } } else { baseValue = int64(value - b.Base) } @@ -2210,6 +2209,10 @@ func (b *bsiGroup) baseValue(op pql.Token, value int64) (baseValue int64, outOfR return baseValue, true } else if value > max { baseValue = int64(max - b.Base) + // Address edge case noted in comments above. + if op == pql.LT { + baseValue++ + } } else { baseValue = int64(value - b.Base) } diff --git a/field_internal_test.go b/field_internal_test.go index e1f20d3d6..db4a3b5e7 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -69,12 +69,12 @@ func TestBSIGroup_BaseValue(t *testing.T) { {b0, pql.LT, 5, 105, false}, {b0, pql.LT, -8, 92, false}, {b0, pql.LT, -108, -8, false}, - {b0, pql.LT, 1005, 1023, false}, + {b0, pql.LT, 1005, 1024, false}, {b0, pql.LT, 0, 100, false}, {b1, pql.LT, 5, 5, false}, {b1, pql.LT, -8, -8, false}, - {b1, pql.LT, 1005, 255, false}, + {b1, pql.LT, 1005, 256, false}, {b1, pql.LT, 0, 0, false}, {b2, pql.LT, 5, -95, false}, @@ -92,7 +92,7 @@ func TestBSIGroup_BaseValue(t *testing.T) { {b1, pql.GT, -8, -8, false}, {b1, pql.GT, 1005, 0, true}, {b1, pql.GT, 0, 0, false}, - {b1, pql.GT, -300, -255, false}, + {b1, pql.GT, -300, -256, false}, {b2, pql.GT, 5, -95, false}, {b2, pql.GT, -8, -108, false}, diff --git a/fragment.go b/fragment.go index 6a801bc50..81f644e3c 100644 --- a/fragment.go +++ b/fragment.go @@ -1242,7 +1242,6 @@ func (f *fragment) rangeLTUnsigned(filter *Row, bitDepth uint, predicate uint64, // if the predicate is larger than all representable numbers given // our bitDepth... then just return everything. if msb(predicate) > bitDepth { - return filter, nil }