diff --git a/executor_test.go b/executor_test.go index bdcef2ee8..f81209757 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1067,6 +1067,64 @@ func TestExecutor(t *testing.T) { }) } }) + + // This is a regression test which checks that queries over a time + // range that encompass all available time views do not default to + // using the standard view. This can be incorrect in the case + // where some time views have been deleted. + t.Run("TimeQueriesFullRange", func(t *testing.T) { + ts := func(t time.Time) int64 { + return t.Unix() * 1e+9 + } + indexName := "tq_range" + c.CreateField(t, indexName, pilosa.IndexOptions{Keys: true, TrackExistence: true}, "f1", pilosa.OptFieldKeys(), pilosa.OptFieldTypeTime(pilosa.TimeQuantum("D"), "0")) + c.ImportTimeQuantumKey(t, indexName, "f1", []test.TimeQuantumKey{ + // from edge cases + {ColKey: "C1", RowKey: "R1", Ts: ts(time.Date(2022, 1, 10, 0, 0, 0, 0, time.UTC))}, + {ColKey: "C2", RowKey: "R1", Ts: ts(time.Date(2022, 1, 11, 0, 0, 0, 0, time.UTC))}, + {ColKey: "C3", RowKey: "R1", Ts: ts(time.Date(2022, 1, 12, 0, 0, 0, 0, time.UTC))}, + }) + c.ImportKeyKey(t, indexName, "f1", [][2]string{ + {"R1", "C2"}, + {"R1", "C3"}, + {"R1", "C4"}, + {"R1", "C5"}, + {"R1", "C6"}, + }) + + tr := c.QueryGRPC(t, indexName, "Row(f1=R1, from='2022-01-10', to='2022-01-13')") + csvString, err := tableResponseToCSVString(tr) + if err != nil { + t.Fatalf("converting to CSV: %v", err) + } + expected := ` +_id +C1 +C3 +C2 +`[1:] + // check that an unqualified query does use the standard view + if csvString != expected { + t.Fatalf("expected:\n%s\ngot:\n%s\n", expected, csvString) + } + tr = c.QueryGRPC(t, indexName, "Row(f1=R1)") + csvString, err = tableResponseToCSVString(tr) + if err != nil { + t.Fatalf("converting to CSV: %v", err) + } + expected = ` +_id +C1 +C3 +C2 +C5 +C4 +C6 +`[1:] + if csvString != expected { + t.Fatalf("expected:\n%s\ngot:\n%s\n", expected, csvString) + } + }) } func runCallTest(c *test.Cluster, t *testing.T, writeQuery string, readQueries []string, indexOptions *pilosa.IndexOptions, fieldOption ...pilosa.FieldOption) []pilosa.QueryResponse { @@ -5076,10 +5134,6 @@ func TestExecutor_Execute_Rows(t *testing.T) { } } -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) diff --git a/field.go b/field.go index 988a79186..5eae96ca6 100644 --- a/field.go +++ b/field.go @@ -968,17 +968,18 @@ func (f *Field) TimeQuantum() TimeQuantum { return f.options.TimeQuantum } -// viewsByTimeRange is a wrapper on the non-method viewsByTimeRange, which -// computes views for a specific field for a given time range. The difference -// is that, as a Field operation, it can return "standard" for a view that -// covers the whole time range, if the field supports a standard view, and -// can automatically coerce from/to times to match the actual range present -// in the field. +// viewsByTimeRange is a wrapper on the non-method viewsByTimeRange, +// which computes views for a specific field for a given time +// range. The difference is that, it can return "standard" if from/to +// are not set and can automatically coerce from/to times to match the +// actual range present in the field. func (f *Field) viewsByTimeRange(from, to time.Time) (views []string, err error) { - // If we can't find time views at all, we'll yield "standard" regardless. - // It's the least-bad answer, I think. + // If we can't find time views at all, we'll yield "standard" + // regardless. It's the least-bad answer, I think. Also yield + // "standard" if from and to were both not set and there is a + // standard view. q := f.TimeQuantum() - if q == "" { + if q == "" || (from.IsZero() && to.IsZero() && !f.options.NoStandardView) { return []string{viewStandard}, nil } @@ -991,10 +992,9 @@ func (f *Field) viewsByTimeRange(from, to time.Time) (views []string, err error) // If min/max are empty, there were no time views. if min == "" || max == "" { - return []string{viewStandard}, nil + return []string{}, nil } - wasZero := from.IsZero() && to.IsZero() // Convert min/max from string to time.Time. minTime, err := timeOfView(min, false) if err != nil { @@ -1011,9 +1011,6 @@ func (f *Field) viewsByTimeRange(from, to time.Time) (views []string, err error) if to.IsZero() || to.After(maxTime) { to = maxTime } - if (wasZero || (from == minTime && to == maxTime)) && !f.Options().NoStandardView { - return []string{viewStandard}, nil - } return viewsByTimeRange(viewStandard, from, to, q), nil } diff --git a/field_internal_test.go b/field_internal_test.go index 268ca95d6..01f0bed0d 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -935,8 +935,8 @@ func TestFieldViewsByTimeRange(t *testing.T) { from, to string expected []string }{ - {"", "", []string{"standard"}}, - {"2020-12-31T00:00", "2023-01-03T00:00", []string{"standard"}}, + {"", "", []string{viewStandard}}, // this is interpreted as no time range being specified at all + {"2020-12-31T00:00", "2023-01-03T00:00", []string{"standard_2021", "standard_2022"}}, {"2021-01-01T00:00", "2022-01-01T00:00", []string{"standard_2021"}}, {"2021-01-01T00:00", "2022-01-02T00:00", []string{"standard_2021", "standard_20220101"}}, {"", "2022-01-02T00:00", []string{"standard_2021", "standard_20220101"}},