From 93b60482639ae0d874985bfe4a2e2e66068ffa51 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Thu, 10 Jan 2019 16:12:54 -0600 Subject: [PATCH 1/8] add verbose flag to circle ci race test to help debug timeout --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 08b615b3b..a27e34be1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -50,7 +50,7 @@ jobs: - *fast-checkout - run: sudo apt-get install lsof - run: - command: make test TESTFLAGS="-race -timeout=30m" + command: make test TESTFLAGS="-race -v -timeout=30m" no_output_timeout: 30m test-golang-1.11-386: <<: *base-test From 0b1fb73b145046aac324013f7ad17fb804df5ee5 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Thu, 10 Jan 2019 11:00:10 -0600 Subject: [PATCH 2/8] add a test for groupby filter with RangeLTLT --- pql/pqlpeg_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pql/pqlpeg_test.go b/pql/pqlpeg_test.go index 0c3a83bb2..c49c4647a 100644 --- a/pql/pqlpeg_test.go +++ b/pql/pqlpeg_test.go @@ -629,6 +629,26 @@ func TestPQLDeepEquality(t *testing.T) { {Name: "Rows"}, }, }}, + { + name: "GroupByFilterRangeLTLT", + call: "GroupBy(Rows(), filter=Row(4 < a < 9))", + exp: &Call{ + Name: "GroupBy", + Args: map[string]interface{}{ + "filter": &Call{ + Name: "Row", + Args: map[string]interface{}{ + "a": &Condition{ + Op: BETWEEN, + Value: []interface{}{int64(5), int64(9)}, + }, + }, + }, + }, + Children: []*Call{ + {Name: "Rows"}, + }, + }}, } for i, test := range tests { From f7e3296f620784e886a0f56de5d7815538605810 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Fri, 11 Jan 2019 09:20:09 -0600 Subject: [PATCH 3/8] fixes a bug on upper end of bsi range queries --- executor_test.go | 42 +++++++++++++++++++++++++++++++++++------- pql/ast.go | 4 ++-- pql/pqlpeg_test.go | 10 +++++----- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/executor_test.go b/executor_test.go index 11fcd32bd..19f39dc7f 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1901,16 +1901,44 @@ func TestExecutor_Execute_Row_BSIGroup(t *testing.T) { }) t.Run("BETWEEN", func(t *testing.T) { - if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(0 < other < 1000)`}); err != nil { - t.Fatal(err) - } else if !reflect.DeepEqual([]uint64{0}, result.Results[0].(*pilosa.Row).Columns()) { - t.Fatalf("unexpected result: %s", spew.Sdump(result)) + tests := []struct { + q string + exp bool + }{ + {q: `Row(0 < other < 1000)`, exp: false}, + {q: `Row(0 <= other < 1000)`, exp: false}, + {q: `Row(0 <= other <= 1000)`, exp: true}, + {q: `Row(0 < other <= 1000)`, exp: true}, + + {q: `Row(1000 < other < 1000)`, exp: false}, + {q: `Row(1000 <= other < 1000)`, exp: false}, + {q: `Row(1000 <= other <= 1000)`, exp: true}, + {q: `Row(1000 < other <= 1000)`, exp: false}, + + {q: `Row(1000 < other < 2000)`, exp: false}, + {q: `Row(1000 <= other < 2000)`, exp: true}, + {q: `Row(1000 <= other <= 2000)`, exp: true}, + {q: `Row(1000 < other <= 2000)`, exp: false}, } + for i, test := range tests { + t.Run(fmt.Sprintf("#%d_%s", i, test.q), func(t *testing.T) { + var expected = []uint64{} + if test.exp { + expected = []uint64{0} + } + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: test.q}); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(expected, result.Results[0].(*pilosa.Row).Columns()) { + t.Fatalf("unexpected result for query: %s", test.q) + } + }) + } + }) // Ensure that the NotNull code path gets run. t.Run("NotNull", func(t *testing.T) { - if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(-1 < other < 1000)`}); err != nil { + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(0 <= other <= 1000)`}); err != nil { t.Fatal(err) } else if !reflect.DeepEqual([]uint64{0}, result.Results[0].(*pilosa.Row).Columns()) { t.Fatalf("unexpected result: %s", spew.Sdump(result)) @@ -2069,14 +2097,14 @@ func TestExecutor_Execute_Range_BSIGroup_Deprecated(t *testing.T) { t.Run("BETWEEN", func(t *testing.T) { if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Range(0 < other < 1000)`}); err != nil { t.Fatal(err) - } else if !reflect.DeepEqual([]uint64{0}, result.Results[0].(*pilosa.Row).Columns()) { + } else if !reflect.DeepEqual([]uint64{}, result.Results[0].(*pilosa.Row).Columns()) { t.Fatalf("unexpected result: %s", spew.Sdump(result)) } }) // Ensure that the NotNull code path gets run. t.Run("NotNull", func(t *testing.T) { - if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Range(-1 < other < 1000)`}); err != nil { + if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Range(0 <= other <= 1000)`}); err != nil { t.Fatal(err) } else if !reflect.DeepEqual([]uint64{0}, result.Results[0].(*pilosa.Row).Columns()) { t.Fatalf("unexpected result: %s", spew.Sdump(result)) diff --git a/pql/ast.go b/pql/ast.go index 0985ab8a5..8b23c708b 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -91,8 +91,8 @@ func (q *Query) endConditional() { if q.conditional[1] == "<" { low++ } - if q.conditional[3] == "<=" { - high++ + if q.conditional[3] == "<" { + high-- } elem := q.lastCallStackElem() diff --git a/pql/pqlpeg_test.go b/pql/pqlpeg_test.go index c49c4647a..3785bb713 100644 --- a/pql/pqlpeg_test.go +++ b/pql/pqlpeg_test.go @@ -501,7 +501,7 @@ func TestPQLDeepEquality(t *testing.T) { Args: map[string]interface{}{ "a": &Condition{ Op: BETWEEN, - Value: []interface{}{int64(4), int64(9)}, + Value: []interface{}{int64(4), int64(8)}, }, }, }}, @@ -513,7 +513,7 @@ func TestPQLDeepEquality(t *testing.T) { Args: map[string]interface{}{ "a": &Condition{ Op: BETWEEN, - Value: []interface{}{int64(5), int64(9)}, + Value: []interface{}{int64(5), int64(8)}, }, }, }}, @@ -525,7 +525,7 @@ func TestPQLDeepEquality(t *testing.T) { Args: map[string]interface{}{ "a": &Condition{ Op: BETWEEN, - Value: []interface{}{int64(4), int64(10)}, + Value: []interface{}{int64(4), int64(9)}, }, }, }}, @@ -537,7 +537,7 @@ func TestPQLDeepEquality(t *testing.T) { Args: map[string]interface{}{ "a": &Condition{ Op: BETWEEN, - Value: []interface{}{int64(5), int64(10)}, + Value: []interface{}{int64(5), int64(9)}, }, }, }}, @@ -640,7 +640,7 @@ func TestPQLDeepEquality(t *testing.T) { Args: map[string]interface{}{ "a": &Condition{ Op: BETWEEN, - Value: []interface{}{int64(5), int64(9)}, + Value: []interface{}{int64(5), int64(8)}, }, }, }, From 9f6d489be85ca59fd39feab81dd16b6f0d0fa3f4 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Mon, 14 Jan 2019 14:39:08 +0300 Subject: [PATCH 4/8] fixes #1823. Updates tests and docs for row range --- docs/query-language.md | 9 ++++----- executor.go | 5 ++--- executor_test.go | 16 +++++++++++++++- pql/pqlpeg_test.go | 8 ++++++++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/docs/query-language.md b/docs/query-language.md index 5eb4ef2d6..73e1cab7d 100644 --- a/docs/query-language.md +++ b/docs/query-language.md @@ -341,13 +341,12 @@ Row(stargazer=1) **Spec:** ``` -Row(=, , ) +Row(=, from=, to=) ``` **Description:** -Similar to `Row`, but only returns bits which were set with timestamps -between the given `start` (first) and `end` (second) timestamps. +Similar to `Row`, but only returns bits which were set with timestamps between the given `from` (inclusive) and `to` (exclusive) timestamps. Both `from` and `to` parameters are optional. The default for `to` timestamp is current time + 1 day. If a later end timestamp is required, specify it explicitly. **Result Type:** object with attrs and bits @@ -356,7 +355,7 @@ between the given `start` (first) and `end` (second) timestamps. Query all columns with a bit set in row 1 of a field (repositories that a user has starred), within a date range: ```request -Row(stargazer=1, 2010-01-01T00:00, 2017-03-02T03:00) +Row(stargazer=1, from='2010-01-01T00:00', to='2017-03-02T03:00') ``` ```response {{"attrs":{},"columns":[10]} @@ -836,7 +835,7 @@ GroupBy(, [RowsCall...], limit=, filter=) GroupBy returns the count of the intersection of every combination of rows taking one row each from the specified `Rows` calls. It returns only those -combinations for which the count is greater than 0. +combinations for which the count is greater than 0. The optional `filter` argument takes any type of `Row` query (e.g. Row, Union, Intersect, etc.) which will be intersected with each result prior to returning diff --git a/executor.go b/executor.go index b9f7c31ca..f68fec726 100644 --- a/executor.go +++ b/executor.go @@ -1253,9 +1253,8 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal // Set maximum "to" value if only "from" is set. We don't need to worry // about setting the minimum "from" since it is the zero value if omitted. if toTime.IsZero() { - // This is the maximum comparable time.Time value. - // https://stackoverflow.com/a/32620397 - toTime = time.Unix(1<<63-62135596801, 999999999) + // Set the end timestamp to current time + 1 day, in order to account for timezone differences. + toTime = time.Now().AddDate(0, 0, 1) } // Union bitmaps across all time-based views. diff --git a/executor_test.go b/executor_test.go index 19f39dc7f..034208485 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1493,6 +1493,8 @@ func TestExecutor_Execute_Row_Range(t *testing.T) { Set(2, f=10, 2001-01-01T00:00)` readQueries := []string{ `Row(f=1, from=1999-12-31T00:00, to=2002-01-01T03:00)`, + `Row(f=1, from=1999-12-31T00:00)`, + `Row(f=1, to=2002-01-01T02:00)`, `Clear( 2, f=1)`, `Row(f=1, from=1999-12-31T00:00, to=2002-01-01T03:00)`, } @@ -1505,8 +1507,20 @@ func TestExecutor_Execute_Row_Range(t *testing.T) { } }) + t.Run("From", func(t *testing.T) { + if columns := responses[1].Results[0].(*pilosa.Row).Columns(); !reflect.DeepEqual(columns, []uint64{2, 3, 4, 5, 6, 7}) { + t.Fatalf("unexpected columns: %+v", columns) + } + }) + + t.Run("To", func(t *testing.T) { + if columns := responses[2].Results[0].(*pilosa.Row).Columns(); !reflect.DeepEqual(columns, []uint64{2, 3, 4, 5, 6}) { + t.Fatalf("unexpected columns: %+v", columns) + } + }) + t.Run("Clear", func(t *testing.T) { - if columns := responses[2].Results[0].(*pilosa.Row).Columns(); !reflect.DeepEqual(columns, []uint64{3, 4, 5, 6, 7}) { + if columns := responses[4].Results[0].(*pilosa.Row).Columns(); !reflect.DeepEqual(columns, []uint64{3, 4, 5, 6, 7}) { t.Fatalf("unexpected columns: %+v", columns) } }) diff --git a/pql/pqlpeg_test.go b/pql/pqlpeg_test.go index 3785bb713..54f73b809 100644 --- a/pql/pqlpeg_test.go +++ b/pql/pqlpeg_test.go @@ -248,6 +248,14 @@ func TestPEGWorking(t *testing.T) { name: "RangeTimeQuotes", input: `Row(a=4, from='2010-07-04T00:00', to="2010-08-04T00:00")`, ncalls: 1}, + { + name: "RangeTimeFromQuotes", + input: `Row(a=4, from='2010-07-04T00:00')`, + ncalls: 1}, + { + name: "RangeTimeToQuotes", + input: `Row(a=4, to="2010-08-04T00:00")`, + ncalls: 1}, { name: "Dashed Frame", input: "Set(1, my-frame=9)", From 600b39e4e575e76bc18224f8ba4cac9480be3459 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Mon, 14 Jan 2019 23:40:31 +0300 Subject: [PATCH 5/8] updates row range test with a timestamp > the default end timestamp --- executor_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/executor_test.go b/executor_test.go index 034208485..e8472bbe4 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1480,17 +1480,21 @@ func TestExecutor_Execute_Sum(t *testing.T) { // Ensure a range query can be executed. func TestExecutor_Execute_Row_Range(t *testing.T) { t.Run("RowIDColumnID", func(t *testing.T) { - writeQuery := ` + // Create a timestamp just out of the current date + 1 day timestamp (default end timestamp). + nextDayExclusive := time.Now().AddDate(0, 0, 1).Add(1 * time.Second) + + writeQuery := fmt.Sprintf(` Set(2, f=1, 1999-12-31T00:00) Set(3, f=1, 2000-01-01T00:00) Set(4, f=1, 2000-01-02T00:00) Set(5, f=1, 2000-02-01T00:00) Set(6, f=1, 2001-01-01T00:00) Set(7, f=1, 2002-01-01T02:00) + Set(8, f=1, %s) Set(2, f=1, 1999-12-30T00:00) Set(2, f=1, 2002-02-01T00:00) - Set(2, f=10, 2001-01-01T00:00)` + Set(2, f=10, 2001-01-01T00:00)`, nextDayExclusive.Format("2006-01-02T15:04")) readQueries := []string{ `Row(f=1, from=1999-12-31T00:00, to=2002-01-01T03:00)`, `Row(f=1, from=1999-12-31T00:00)`, From d75e9eb772e9a39d3a22b00e3683a1a5972ef2a4 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Mon, 14 Jan 2019 23:45:58 +0300 Subject: [PATCH 6/8] updated row range test --- executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor_test.go b/executor_test.go index e8472bbe4..19a660b50 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1481,7 +1481,7 @@ func TestExecutor_Execute_Sum(t *testing.T) { func TestExecutor_Execute_Row_Range(t *testing.T) { t.Run("RowIDColumnID", func(t *testing.T) { // Create a timestamp just out of the current date + 1 day timestamp (default end timestamp). - nextDayExclusive := time.Now().AddDate(0, 0, 1).Add(1 * time.Second) + nextDayExclusive := time.Now().AddDate(0, 0, 1).Add(1 * time.Hour) writeQuery := fmt.Sprintf(` Set(2, f=1, 1999-12-31T00:00) From 76de81dacffb36e113dfb66d99a7faa87b9c09b4 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Mon, 14 Jan 2019 23:46:34 +0300 Subject: [PATCH 7/8] updated row range test --- executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor_test.go b/executor_test.go index 19a660b50..0d42af8c5 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1481,7 +1481,7 @@ func TestExecutor_Execute_Sum(t *testing.T) { func TestExecutor_Execute_Row_Range(t *testing.T) { t.Run("RowIDColumnID", func(t *testing.T) { // Create a timestamp just out of the current date + 1 day timestamp (default end timestamp). - nextDayExclusive := time.Now().AddDate(0, 0, 1).Add(1 * time.Hour) + nextDayExclusive := time.Now().AddDate(0, 0, 2) writeQuery := fmt.Sprintf(` Set(2, f=1, 1999-12-31T00:00) From 0a8bd6548be945653880b942815ea9febcc3122b Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Mon, 14 Jan 2019 17:09:30 -0600 Subject: [PATCH 8/8] don't delete test fragment data (part of repo) --- fragment_internal_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/fragment_internal_test.go b/fragment_internal_test.go index cca88dda1..f35cec46d 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1142,7 +1142,7 @@ func BenchmarkFragment_Blocks(b *testing.B) { if err := f.Open(); err != nil { b.Fatal(err) } - defer f.Clean(b) + defer f.CleanKeep(b) // Reset timer and execute benchmark. b.ResetTimer() @@ -1671,7 +1671,7 @@ func BenchmarkFragment_Snapshot(b *testing.B) { if err := f.Open(); err != nil { b.Fatal(err) } - defer f.Clean(b) + defer f.CleanKeep(b) b.ResetTimer() // Reset timer and execute benchmark. @@ -2030,6 +2030,20 @@ func (f *fragment) Clean(t testing.TB) { } } +// CleanKeep is just like Clean(), but it doesn't remove the +// fragment file (note that it DOES remove the cache file). +func (f *fragment) CleanKeep(t testing.TB) { + errc := f.Close() + errp := os.Remove(f.cachePath()) + if errc != nil { + t.Fatal("closing fragment: ", errc, errp) + } + // not all fragments have cache files + if errp != nil && !os.IsNotExist(errp) { + t.Fatalf("cleaning up fragment cache: %v", errp) + } +} + // mustOpenFragment returns a new instance of Fragment with a temporary path. func mustOpenFragment(index, field, view string, shard uint64, cacheType string) *fragment { file, err := ioutil.TempFile(TempDir, "pilosa-fragment-")