From 5f4545eeb2ddee9a3cbe943f53e0cea7bb66fb24 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Tue, 21 Nov 2017 13:43:44 -0600
Subject: [PATCH] Fix edge case with Range() calls outside field Min/Max. Fixes
 #876.

---
 executor.go      |  6 ++++++
 executor_test.go | 27 +++++++++++++++++++++++++++
 frame.go         |  4 +++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/executor.go b/executor.go
index 2f5acf3c0..b165eb4da 100644
--- a/executor.go
+++ b/executor.go
@@ -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())
diff --git a/executor_test.go b/executor_test.go
index 4eb81e492..dd8725298 100644
--- a/executor_test.go
+++ b/executor_test.go
@@ -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)
diff --git a/frame.go b/frame.go
index 3da55cc93..9940c120e 100644
--- a/frame.go
+++ b/frame.go
@@ -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 {