From 07ae881967dd2aba1b20e90c7db18734dd1ad2a2 Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 5 Oct 2017 11:52:14 -0500 Subject: [PATCH 1/4] Add FieldNotNull for more efficient BETWEEN queries --- executor.go | 10 ++++++++++ executor_test.go | 17 +++++++++++++++++ fragment.go | 5 +++++ 3 files changed, 32 insertions(+) diff --git a/executor.go b/executor.go index ae960a5e1..62ac7e7ff 100644 --- a/executor.go +++ b/executor.go @@ -727,6 +727,10 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * return nil, errors.New("Range(): BETWEEN condition requires exactly two integer values") } + // The reason we don't just call: + // return f.FieldRangeBetween(fieldName, predicates[0], predicates[1]) + // here is because we need the call to be slice-specific. + // Find field. field := f.Field(fieldName) if field == nil { @@ -744,6 +748,12 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * return NewBitmap(), nil } + // If the query is asking for the entire valid range, just return + // the not-null bitmap for the field. + if predicates[0] <= field.Min && predicates[1] >= field.Max { + return frag.FieldNotNull(field.BitDepth()) + } + return frag.FieldRangeBetween(field.BitDepth(), baseValueMin, baseValueMax) } else { diff --git a/executor_test.go b/executor_test.go index 482903bc2..2a5b90e4b 100644 --- a/executor_test.go +++ b/executor_test.go @@ -796,6 +796,23 @@ func TestExecutor_Execute_FieldRange(t *testing.T) { } }) + t.Run("BETWEEN", func(t *testing.T) { + if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=other, foo >< [1, 1000])`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual([]uint64{0}, result[0].(*pilosa.Bitmap).Bits()) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } + }) + + // Ensure that the FieldNotNull code path gets run. + t.Run("FieldNotNull", func(t *testing.T) { + if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=other, foo >< [0, 1000])`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual([]uint64{0}, result[0].(*pilosa.Bitmap).Bits()) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } + }) + t.Run("BelowMin", func(t *testing.T) { if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=f, foo == 0)`), nil, nil); err != nil { t.Fatal(err) diff --git a/fragment.go b/fragment.go index cca8c4ede..896096667 100644 --- a/fragment.go +++ b/fragment.go @@ -730,6 +730,11 @@ func (f *Fragment) fieldRangeGT(bitDepth uint, predicate uint64, allowEquality b return b, nil } +// FieldNotNull returns the not-null row (stored at bitDepth). +func (f *Fragment) FieldNotNull(bitDepth uint) (*Bitmap, error) { + return f.Row(uint64(bitDepth)), nil +} + func (f *Fragment) FieldRangeBetween(bitDepth uint, predicateMin, predicateMax uint64) (*Bitmap, error) { b := f.Row(uint64(bitDepth)) keep1 := NewBitmap() // GTE From 4304d341f773a7f637a318cd35d2cfd3c4c732fb Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 5 Oct 2017 15:02:30 -0500 Subject: [PATCH 2/4] Add support for `field != null` Range query --- executor.go | 25 ++++++++++++++++++++++++- executor_test.go | 8 ++++++++ pql/parser.go | 2 +- pql/parser_test.go | 3 ++- pql/scanner.go | 6 ++++++ pql/scanner_test.go | 1 + pql/token.go | 2 ++ 7 files changed, 44 insertions(+), 3 deletions(-) diff --git a/executor.go b/executor.go index 62ac7e7ff..0b922fedb 100644 --- a/executor.go +++ b/executor.go @@ -715,7 +715,30 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * fieldName, cond = k, vv } - if cond.Op == pql.BETWEEN { + // EQ null (not implemented: flip frag.FieldNotNull with max ColumnID) + // NEQ null frag.FieldNotNull() + // BETWEEN a,b(in) BETWEEN/frag.FieldRangeBetween() + // BETWEEN a,b(out) BETWEEN/frag.FieldNotNull() + // EQ frag.FieldRange + // NEQ (not implemented: frag.FieldRange) + + // Handle `!= null`. + if cond.Op == pql.NEQ && cond.Value == nil { + // Find field. + field := f.Field(fieldName) + if field == nil { + return nil, ErrFieldNotFound + } + + // Retrieve fragment. + frag := e.Holder.Fragment(index, frame, ViewFieldPrefix+fieldName, slice) + if frag == nil { + return NewBitmap(), nil + } + + return frag.FieldNotNull(field.BitDepth()) + + } else if cond.Op == pql.BETWEEN { predicates, err := cond.IntSliceValue() if err != nil { diff --git a/executor_test.go b/executor_test.go index 2a5b90e4b..83199f318 100644 --- a/executor_test.go +++ b/executor_test.go @@ -764,6 +764,14 @@ func TestExecutor_Execute_FieldRange(t *testing.T) { } }) + t.Run("NEQ", func(t *testing.T) { + if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=other, foo != null)`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual([]uint64{0}, result[0].(*pilosa.Bitmap).Bits()) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } + }) + t.Run("LT", func(t *testing.T) { if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=f, foo < 20)`), nil, nil); err != nil { t.Fatal(err) diff --git a/pql/parser.go b/pql/parser.go index ab5298458..3af0cbc9c 100644 --- a/pql/parser.go +++ b/pql/parser.go @@ -165,7 +165,7 @@ func (p *Parser) parseArgs() (map[string]interface{}, error) { var op Token switch tok, pos, lit := p.scanIgnoreWhitespace(); tok { case ASSIGN: - case EQ, LT, LTE, GT, GTE, BETWEEN: + case EQ, NEQ, LT, LTE, GT, GTE, BETWEEN: op = tok default: return nil, parseErrorf(pos, "expected equals sign or comparison operator, found %q", lit) diff --git a/pql/parser_test.go b/pql/parser_test.go index 266e1de1f..0e2a5c17d 100644 --- a/pql/parser_test.go +++ b/pql/parser_test.go @@ -172,7 +172,7 @@ func TestParser_Parse(t *testing.T) { // Parse with condition arguments. t.Run("WithCondition", func(t *testing.T) { - q, err := pql.ParseString(`MyCall(key=foo, x == 12.25, y >= 100, z >< [4,8])`) + q, err := pql.ParseString(`MyCall(key=foo, x == 12.25, y >= 100, z >< [4,8], m != null)`) if err != nil { t.Fatal(err) } else if !reflect.DeepEqual(q.Calls[0], @@ -183,6 +183,7 @@ func TestParser_Parse(t *testing.T) { "x": &pql.Condition{Op: pql.EQ, Value: 12.25}, "y": &pql.Condition{Op: pql.GTE, Value: int64(100)}, "z": &pql.Condition{Op: pql.BETWEEN, Value: []interface{}{int64(4), int64(8)}}, + "m": &pql.Condition{Op: pql.NEQ, Value: nil}, }, }, ) { diff --git a/pql/scanner.go b/pql/scanner.go index f25abf1f4..5a24b6af2 100644 --- a/pql/scanner.go +++ b/pql/scanner.go @@ -66,6 +66,12 @@ func (s *Scanner) Scan() (tok Token, pos Pos, lit string) { } s.unread() return ASSIGN, pos, string(ch) + case '!': + if next := s.read(); next == '=' { + return NEQ, pos, "!=" + } + s.unread() + return ASSIGN, pos, string(ch) case '<': if next := s.read(); next == '=' { return LTE, pos, "<=" diff --git a/pql/scanner_test.go b/pql/scanner_test.go index 98c13e232..e48896748 100644 --- a/pql/scanner_test.go +++ b/pql/scanner_test.go @@ -38,6 +38,7 @@ func TestScanner_Scan(t *testing.T) { {name: "ASSIGN", s: `=`, tok: pql.ASSIGN, lit: `=`}, {name: "EQ", s: `==`, tok: pql.EQ, lit: `==`}, + {name: "NEQ", s: `!=`, tok: pql.NEQ, lit: `!=`}, {name: "LT", s: `<`, tok: pql.LT, lit: `<`}, {name: "LTE", s: `<=`, tok: pql.LTE, lit: `<=`}, {name: "GT", s: `>`, tok: pql.GT, lit: `>`}, diff --git a/pql/token.go b/pql/token.go index 2870553e4..6997f17af 100644 --- a/pql/token.go +++ b/pql/token.go @@ -39,6 +39,7 @@ const ( ASSIGN // = EQ // == + NEQ // != LT // < LTE // <= GT // > @@ -64,6 +65,7 @@ var tokens = [...]string{ ASSIGN: "=", EQ: "==", + NEQ: "!=", LT: "<", LTE: "<=", GT: ">", From d62ea053b5cf78820ac8d31967a1ae669058c3e9 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 6 Oct 2017 11:32:57 -0500 Subject: [PATCH 3/4] Add support for `field != ` Range query --- executor.go | 2 +- executor_test.go | 7 +++++++ fragment.go | 18 ++++++++++++++++++ fragment_test.go | 23 +++++++++++++++++++++++ frame.go | 2 +- 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 0b922fedb..dd9ffc3a8 100644 --- a/executor.go +++ b/executor.go @@ -720,7 +720,7 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * // BETWEEN a,b(in) BETWEEN/frag.FieldRangeBetween() // BETWEEN a,b(out) BETWEEN/frag.FieldNotNull() // EQ frag.FieldRange - // NEQ (not implemented: frag.FieldRange) + // NEQ frag.FieldRange // Handle `!= null`. if cond.Op == pql.NEQ && cond.Value == nil { diff --git a/executor_test.go b/executor_test.go index 83199f318..130421c00 100644 --- a/executor_test.go +++ b/executor_test.go @@ -765,11 +765,18 @@ func TestExecutor_Execute_FieldRange(t *testing.T) { }) t.Run("NEQ", func(t *testing.T) { + // NEQ null if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=other, foo != null)`), nil, nil); err != nil { t.Fatal(err) } else if !reflect.DeepEqual([]uint64{0}, result[0].(*pilosa.Bitmap).Bits()) { t.Fatalf("unexpected result: %s", spew.Sdump(result)) } + // NEQ + if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=f, foo != 20)`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual([]uint64{SliceWidth, SliceWidth + 1, SliceWidth + 2}, result[0].(*pilosa.Bitmap).Bits()) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } }) t.Run("LT", func(t *testing.T) { diff --git a/fragment.go b/fragment.go index 896096667..263966ad0 100644 --- a/fragment.go +++ b/fragment.go @@ -619,6 +619,8 @@ func (f *Fragment) FieldRange(op pql.Token, bitDepth uint, predicate uint64) (*B switch op { case pql.EQ: return f.fieldRangeEQ(bitDepth, predicate) + case pql.NEQ: + return f.fieldRangeNEQ(bitDepth, predicate) case pql.LT, pql.LTE: return f.fieldRangeLT(bitDepth, predicate, op == pql.LTE) case pql.GT, pql.GTE: @@ -647,6 +649,22 @@ func (f *Fragment) fieldRangeEQ(bitDepth uint, predicate uint64) (*Bitmap, error return b, nil } +func (f *Fragment) fieldRangeNEQ(bitDepth uint, predicate uint64) (*Bitmap, error) { + // Start with set of columns with values set. + b := f.Row(uint64(bitDepth)) + + // Get the equal bitmap. + eq, err := f.fieldRangeEQ(bitDepth, predicate) + if err != nil { + return nil, err + } + + // Not-null minus the equal bitmap. + b = b.Difference(eq) + + return b, nil +} + func (f *Fragment) fieldRangeLT(bitDepth uint, predicate uint64, allowEquality bool) (*Bitmap, error) { keep := NewBitmap() diff --git a/fragment_test.go b/fragment_test.go index 21723dc3b..2ccf538f7 100644 --- a/fragment_test.go +++ b/fragment_test.go @@ -283,6 +283,29 @@ func TestFragment_FieldRange(t *testing.T) { } }) + t.Run("NEQ", func(t *testing.T) { + f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, "") + defer f.Close() + + // Set values. + if _, err := f.SetFieldValue(1000, bitDepth, 382); err != nil { + t.Fatal(err) + } else if _, err := f.SetFieldValue(2000, bitDepth, 300); err != nil { + t.Fatal(err) + } else if _, err := f.SetFieldValue(3000, bitDepth, 2818); err != nil { + t.Fatal(err) + } else if _, err := f.SetFieldValue(4000, bitDepth, 300); err != nil { + t.Fatal(err) + } + + // Query for inequality. + if b, err := f.FieldRange(pql.NEQ, bitDepth, 300); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(b.Bits(), []uint64{1000, 3000}) { + t.Fatalf("unexpected bits: %+v", b.Bits()) + } + }) + t.Run("LT", func(t *testing.T) { f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, "") defer f.Close() diff --git a/frame.go b/frame.go index e7bfd4a40..141772a93 100644 --- a/frame.go +++ b/frame.go @@ -1119,7 +1119,7 @@ func (f *Field) BaseValue(op pql.Token, value int64) (baseValue uint64, outOfRan } else { baseValue = uint64(value - f.Min) } - } else if op == pql.EQ { + } else if op == pql.EQ || op == pql.NEQ { if value < f.Min || value > f.Max { return baseValue, true } From b0afec07611ecb8087361687023b9f5e89cd5207 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 6 Oct 2017 14:47:56 -0500 Subject: [PATCH 4/4] Make sure that outOfRange field predicates return not-null for NEQ queries --- executor.go | 7 ++++++- executor_test.go | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index dd9ffc3a8..a620a1145 100644 --- a/executor.go +++ b/executor.go @@ -794,7 +794,7 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * } baseValue, outOfRange := field.BaseValue(cond.Op, value) - if outOfRange { + if outOfRange && cond.Op != pql.NEQ { return NewBitmap(), nil } @@ -804,6 +804,11 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * return NewBitmap(), nil } + // outOfRange for NEQ should return all not-null. + if outOfRange && cond.Op == pql.NEQ { + return frag.FieldNotNull(field.BitDepth()) + } + f.Stats.Count("range:field", 1, 1.0) return frag.FieldRange(cond.Op, field.BitDepth(), baseValue) } diff --git a/executor_test.go b/executor_test.go index 130421c00..3ccac27ec 100644 --- a/executor_test.go +++ b/executor_test.go @@ -777,6 +777,13 @@ func TestExecutor_Execute_FieldRange(t *testing.T) { } else if !reflect.DeepEqual([]uint64{SliceWidth, SliceWidth + 1, SliceWidth + 2}, result[0].(*pilosa.Bitmap).Bits()) { t.Fatalf("unexpected result: %s", spew.Sdump(result)) } + // NEQ - + if result, err := e.Execute(context.Background(), "i", test.MustParse(`Range(frame=other, foo != -20)`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual([]uint64{0}, result[0].(*pilosa.Bitmap).Bits()) { + //t.Fatalf("unexpected result: %s", spew.Sdump(result)) + t.Fatalf("unexpected result: %s", result[0].(*pilosa.Bitmap).Bits()) + } }) t.Run("LT", func(t *testing.T) {