Fix edge case with Range() calls outside field Min/Max. Fixes #876.

This commit is contained in:
Cody Soyland 2017-11-21 13:43:44 -06:00
parent e7554dca90
commit 5f4545eeb2
3 changed files with 36 additions and 1 deletions

View file

@ -803,6 +803,12 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c *
return NewBitmap(), nil
}
// LT[E] and GT[E] should return all not-null if selected range fully encompases valid field range.
if (cond.Op == pql.LT && value > field.Max) || (cond.Op == pql.LTE && value >= field.Max) ||
(cond.Op == pql.GT && value < field.Min) || (cond.Op == pql.GTE && value <= field.Min) {
return frag.FieldNotNull(field.BitDepth())
}
// outOfRange for NEQ should return all not-null.
if outOfRange && cond.Op == pql.NEQ {
return frag.FieldNotNull(field.BitDepth())

View file

@ -741,6 +741,15 @@ func TestExecutor_Execute_FieldRange(t *testing.T) {
t.Fatal(err)
}
if _, err := idx.CreateFrame("edge", pilosa.FrameOptions{
RangeEnabled: true,
Fields: []*pilosa.Field{
{Name: "foo", Type: pilosa.FieldTypeInt, Min: -100, Max: 100},
},
}); err != nil {
t.Fatal(err)
}
if _, err := e.Execute(context.Background(), "i", test.MustParse(`
SetBit(frame=f, rowID=0, columnID=0)
SetBit(frame=f, rowID=0, columnID=`+strconv.Itoa(SliceWidth+1)+`)
@ -751,6 +760,8 @@ func TestExecutor_Execute_FieldRange(t *testing.T) {
SetFieldValue(frame=f, foo=20, columnID=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(frame=f, foo=60, columnID=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(frame=other, foo=1000, columnID=0)
SetFieldValue(frame=edge, foo=100, columnID=0)
SetFieldValue(frame=edge, foo=-100, columnID=1)
`), nil, nil); err != nil {
t.Fatal(err)
}
@ -850,6 +861,22 @@ func TestExecutor_Execute_FieldRange(t *testing.T) {
}
})
t.Run("LTAboveMax", func(t *testing.T) {
if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=edge, foo < 200)`), nil, nil); err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual([]uint64{0, 1}, result[0].(*pilosa.Bitmap).Bits()) {
t.Fatalf("unexpected result: %s", spew.Sdump(result[0].(*pilosa.Bitmap).Bits()))
}
})
t.Run("GTBelowMin", func(t *testing.T) {
if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=edge, foo > -200)`), nil, nil); err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual([]uint64{0, 1}, result[0].(*pilosa.Bitmap).Bits()) {
t.Fatalf("unexpected result: %s", spew.Sdump(result[0].(*pilosa.Bitmap).Bits()))
}
})
t.Run("ErrFrameNotFound", func(t *testing.T) {
if _, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=bad_frame, foo >= 20)`), nil, nil); err != pilosa.ErrFrameNotFound {
t.Fatal(err)

View file

@ -1108,7 +1108,7 @@ func (f *Field) BitDepth() uint {
// BaseValue adjusts the value to align with the range for Field for a certain
// operation type.
// TODO: there is an edge case for GT and LT where this returns a baseValue
// Note: There is an edge case for GT and LT where this returns a baseValue
// that does not fully encompass the range.
// ex: Field.Min = 0, Field.Max = 1023
// BaseValue(LT, 2000) returns 1023, which will perform "LT 1023" and effectively
@ -1116,6 +1116,8 @@ func (f *Field) BitDepth() uint {
// 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.executeFieldRangeSlice() takes this into account and returns
// `frag.FieldNotNull(field.BitDepth())` in such instances.
func (f *Field) BaseValue(op pql.Token, value int64) (baseValue uint64, outOfRange bool) {
if op == pql.GT || op == pql.GTE {
if value > f.Max {