From 87b3438cb1ffc8a641925da34f9634ba68ac7664 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Fri, 1 Feb 2019 15:30:10 -0600 Subject: [PATCH] add logic to restrict time range to available views --- executor.go | 48 +++++++++++++--- executor_test.go | 38 +++++++------ time.go | 102 ++++++++++++++++++++++++++++++++++ time_internal_test.go | 125 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 27 deletions(-) diff --git a/executor.go b/executor.go index 52972e7d8..15517b7a9 100644 --- a/executor.go +++ b/executor.go @@ -1167,7 +1167,7 @@ func (e *executor) executeRowsShard(_ context.Context, index string, fieldName s var fromTime time.Time if v, ok := c.Args["from"]; ok { if fromTime, err = parseTime(v); err != nil { - return nil, errors.Wrap(err, "determining from time") + return nil, errors.Wrap(err, "parsing from time") } } @@ -1175,21 +1175,51 @@ func (e *executor) executeRowsShard(_ context.Context, index string, fieldName s var toTime time.Time if v, ok := c.Args["to"]; ok { if toTime, err = parseTime(v); err != nil { - return nil, errors.Wrap(err, "determining to time") + return nil, errors.Wrap(err, "parsing to time") } } - if !fromTime.IsZero() && !toTime.IsZero() { + // Calculate the views for a range as long as some piece of the range + // (from/to) are specified, or if there's no standard view to represent + // all dates. + if !fromTime.IsZero() || !toTime.IsZero() || f.options.NoStandardView { // If no quantum exists then return an empty result set. q := f.TimeQuantum() if q == "" { return rowIDs, nil } - // Determine the views based on the specified time range. - views = viewsByTimeRange(viewStandard, fromTime, toTime, q) - } else if f.options.NoStandardView { - return nil, errors.New("Rows() query on time field with no standard view requires a date range") + // Get min/max based on existing views. + var vs []string + for _, v := range f.views() { + vs = append(vs, v.name) + } + min, max := minMaxViews(vs, q) + + // If min/max are empty, there were no time views. + if min == "" || max == "" { + views = []string{} + } else { + // Convert min/max from string to time.Time. + minTime, err := timeOfView(min, false) + if err != nil { + return rowIDs, errors.Wrapf(err, "getting min time from view: %s", min) + } + if fromTime.IsZero() || fromTime.Before(minTime) { + fromTime = minTime + } + + maxTime, err := timeOfView(max, true) + if err != nil { + return rowIDs, errors.Wrapf(err, "getting max time from view: %s", max) + } + if toTime.IsZero() || toTime.After(maxTime) { + toTime = maxTime + } + + // Determine the views based on the specified time range. + views = viewsByTimeRange(viewStandard, fromTime, toTime, q) + } } } @@ -1272,7 +1302,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal var fromTime time.Time if v, ok := c.Args["from"]; ok { if fromTime, err = parseTime(v); err != nil { - return nil, errors.Wrap(err, "determining from time") + return nil, errors.Wrap(err, "parsing from time") } } @@ -1280,7 +1310,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal var toTime time.Time if v, ok := c.Args["to"]; ok { if toTime, err = parseTime(v); err != nil { - return nil, errors.Wrap(err, "determining to time") + return nil, errors.Wrap(err, "parsing to time") } } diff --git a/executor_test.go b/executor_test.go index 20622e270..a36ddcf72 100644 --- a/executor_test.go +++ b/executor_test.go @@ -3131,10 +3131,6 @@ func TestExecutor_Execute_Rows(t *testing.T) { } func TestExecutor_Execute_RowsTime(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - c.CreateField(t, "i", pilosa.IndexOptions{}, "f", pilosa.OptFieldTypeTime(pilosa.TimeQuantum("YMD"), true)) - writeQuery := fmt.Sprintf(` Set(9, f=1, 2001-01-01T00:00) Set(9, f=2, 2002-01-01T00:00) @@ -3144,25 +3140,20 @@ func TestExecutor_Execute_RowsTime(t *testing.T) { Set(%d, f=13, 2003-02-02T00:00) `, pilosa.ShardWidth+9) readQueries := []string{ - `Rows(field=f, from=1999-12-31T00:00, to=2002-01-01T03:00)`, - `Rows(field=f, from=2002-01-01T00:00, to=2004-01-01T00:00)`, - `Rows(field=f, from=1990-01-01T00:00, to=1999-01-01T00:00)`, + `Rows(f, from=1999-12-31T00:00, to=2002-01-01T03:00)`, + `Rows(f, from=2002-01-01T00:00, to=2004-01-01T00:00)`, + `Rows(f, from=1990-01-01T00:00, to=1999-01-01T00:00)`, + `Rows(f)`, + `Rows(f, from=2002-01-01T00:00)`, + `Rows(f, to=2003-02-03T00:00)`, } expResults := [][]uint64{ {1}, {2, 3, 13}, {}, - } - - // Make sure that date range is enforced when there's no standard view - // or if only a partial date range is provided. - exp := "executing: Rows() query on time field with no standard view requires a date range" - if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Rows(field=f)`}); err == nil || err.Error() != exp { - t.Fatalf("expected error: %s", exp) - } else if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Rows(field=f, from=1999-12-31T00:00)`}); err == nil || err.Error() != exp { - t.Fatalf("expected error: %s", exp) - } else if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Rows(field=f, to=1999-12-31T00:00)`}); err == nil || err.Error() != exp { - t.Fatalf("expected error: %s", exp) + {1, 2, 3, 4, 13}, + {2, 3, 4, 13}, + {1, 2, 3, 13}, } responses := runCallTest(t, writeQuery, readQueries, @@ -3177,6 +3168,17 @@ func TestExecutor_Execute_RowsTime(t *testing.T) { } } +// Ensure that an empty time field returns empty Rows(). +func TestExecutor_Execute_RowsTimeEmpty(t *testing.T) { + c := test.MustRunCluster(t, 1) + defer c.Close() + c.CreateField(t, "i", pilosa.IndexOptions{}, "x", pilosa.OptFieldTypeTime(pilosa.TimeQuantum("YMD"), true)) + rows := c.Query(t, "i", `Rows(x, from=1999-12-31T00:00, to=2002-01-01T03:00)`).Results[0].(pilosa.RowIdentifiers).Rows + if !reflect.DeepEqual(rows, []uint64{}) { + t.Fatalf("unexpected rows: %+v", rows) + } +} + func TestExecutor_Execute_Query_Error(t *testing.T) { c := test.MustRunCluster(t, 1) defer c.Close() diff --git a/time.go b/time.go index 50c15df0a..75ca543ea 100644 --- a/time.go +++ b/time.go @@ -17,6 +17,7 @@ package pilosa import ( "errors" "fmt" + "sort" "strings" "time" ) @@ -215,6 +216,7 @@ func nextDayGTE(t time.Time, end time.Time) bool { return end.After(next) } +// parseTime parses a string or int64 into a time.Time value. func parseTime(t interface{}) (time.Time, error) { var err error var calcTime time.Time @@ -230,3 +232,103 @@ func parseTime(t interface{}) (time.Time, error) { } return calcTime, nil } + +// minMaxViews returns the min and max view from a list of views +// with a time quantum taken into consideration. It assumes that +// all views represent the same base view name (the logic depends +// on the views sorting correctly in alphabetical order). +func minMaxViews(views []string, q TimeQuantum) (min string, max string) { + // Sort the list of views. + sort.Strings(views) + + // Determine the least significant quantum and set that as the + // number of string characters to compare against. + var chars int + if q.HasYear() { + chars = 4 + } else if q.HasMonth() { + chars = 6 + } else if q.HasDay() { + chars = 8 + } else if q.HasHour() { + chars = 10 + } + + // min: get the first view with the matching number of time chars. + for _, v := range views { + if len(viewTimePart(v)) == chars { + min = v + break + } + } + + // max: get the first view (from the end) with the matching number of time chars. + for i := len(views) - 1; i >= 0; i-- { + if len(viewTimePart(views[i])) == chars { + max = views[i] + break + } + } + + return min, max +} + +// timeOfView returns a valid time.Time based on the view string. +// For upper bound use, the result can be adjusted by one by setting +// the `adj` argument to `true`. +func timeOfView(v string, adj bool) (time.Time, error) { + if v == "" { + return time.Time{}, nil + } + + layout := "2006010203" + timePart := viewTimePart(v) + + switch len(timePart) { + case 4: // year + t, err := time.Parse(layout[:4], timePart) + if err != nil { + return time.Time{}, err + } + if adj { + t = t.AddDate(1, 0, 0) + } + return t, nil + case 6: // month + t, err := time.Parse(layout[:6], timePart) + if err != nil { + return time.Time{}, err + } + if adj { + t = addMonth(t) + } + return t, nil + case 8: // day + t, err := time.Parse(layout[:8], timePart) + if err != nil { + return time.Time{}, err + } + if adj { + t = t.AddDate(0, 0, 1) + } + return t, nil + case 10: // hour + t, err := time.Parse(layout[:10], timePart) + if err != nil { + return time.Time{}, err + } + if adj { + t = t.Add(time.Hour) + } + return t, nil + } + + return time.Time{}, fmt.Errorf("invalid time format on view: %s", v) +} + +// viewTimePart returns the time portion of a string view name. +// e.g. the view "string_201901" would return "201901". +func viewTimePart(v string) string { + parts := strings.Split(v, "_") + return parts[len(parts)-1] +} diff --git a/time_internal_test.go b/time_internal_test.go index 18d5933be..58079380b 100644 --- a/time_internal_test.go +++ b/time_internal_test.go @@ -165,6 +165,131 @@ func TestViewsByTimeRange(t *testing.T) { }) } +func TestMinMaxViews(t *testing.T) { + t.Run("Combos", func(t *testing.T) { + tests := []struct { + views []string + q TimeQuantum + min string + max string + }{ + { + []string{""}, + mustParseTimeQuantum("Y"), + "", + "", + }, + { + []string{"std_2019", "std_2020", "std_202002", "std_202002", "std_2022"}, + mustParseTimeQuantum("Y"), + "std_2019", + "std_2022", + }, + { + []string{"std_201902", "std_201901"}, + mustParseTimeQuantum("M"), + "std_201901", + "std_201902", + }, + { + []string{"std_201902", "std_201901"}, + mustParseTimeQuantum("D"), + "", + "", + }, + { + []string{"std_20190201"}, + mustParseTimeQuantum("D"), + "std_20190201", + "std_20190201", + }, + { + []string{"foo", "bar"}, + mustParseTimeQuantum("D"), + "", + "", + }, + } + for i, test := range tests { + if min, max := minMaxViews(test.views, test.q); min != test.min { + t.Errorf("test %d expected min: %v, but got: %v", i, test.min, min) + } else if max != test.max { + t.Errorf("test %d expected max: %v, but got: %v", i, test.max, max) + } + } + }) +} + +func TestTimeOfView(t *testing.T) { + t.Run("Combos", func(t *testing.T) { + tests := []struct { + view string + exp time.Time + expAdj time.Time + expErr string + }{ + { + "std_2019", + time.Date(2019, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + "", + }, + { + "std_201902", + time.Date(2019, 2, 1, 0, 0, 0, 0, time.UTC), + time.Date(2019, 3, 1, 0, 0, 0, 0, time.UTC), + "", + }, + { + "std_20190203", + time.Date(2019, 2, 3, 0, 0, 0, 0, time.UTC), + time.Date(2019, 2, 4, 0, 0, 0, 0, time.UTC), + "", + }, + { + "std_2019020308", + time.Date(2019, 2, 3, 8, 0, 0, 0, time.UTC), + time.Date(2019, 2, 3, 9, 0, 0, 0, time.UTC), + "", + }, + { + "foo", + time.Time{}, + time.Time{}, + "invalid time format on view: foo", + }, + { + "std_201902030801", + time.Time{}, + time.Time{}, + "invalid time format on view: std_201902030801", + }, + } + for i, test := range tests { + // adj: false + if tm, err := timeOfView(test.view, false); err != nil { + if err.Error() != test.expErr { + t.Errorf("test %d got unexpected error: %s", i, err) + } + } else if test.expErr != "" { + t.Errorf("test %d expected error: %s but got none", i, test.expErr) + } else if tm != test.exp { + t.Errorf("test %d expected time: %v, but got: %v", i, test.exp, tm) + } + // adj: true + if tm, err := timeOfView(test.view, true); err != nil { + if err.Error() != test.expErr { + t.Errorf("test %d got unexpected error: %s", i, err) + } + } else if test.expErr != "" { + t.Errorf("test %d expected error: %s but got none", i, test.expErr) + } else if tm != test.expAdj { + t.Errorf("test %d expected time: %v, but got: %v", i, test.expAdj, tm) + } + } + }) +} + // defaultTimeLayout is the time layout used by the tests. const defaultTimeLayout = "2006-01-02 15:04"