dont default to standard view for some time range queries

previously, we would use the standard view if the query seemed to
cover all the views we had, or if we didn't seem to have any time
views. This is unintuitive if some views have been deleted (which
comes up a lot more often with TTL!). It's also unintuitive if you
know you haven't set any data w/ a timestamp and your query that
specifies a time range returns any data.
This commit is contained in:
Matthew Jaffee 2022-05-03 16:46:12 -05:00 • committed by Matthew Jaffee
parent 0d50bd2890
commit a093099a8c
3 changed files with 71 additions and 20 deletions

View file

@ -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)

View file

@ -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
}

View file

@ -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"}},