don't segfault for me, empty distinct results on timestamp field

There's an obvious bug, plus another bug that I hit trying to reproduce
the first bug, plus another... it's a long story.

Basically: If you get nothing back from executeDistinctShardBSI on a
Timestamp field, the request for a large enough pool of strings to hold
timestamp conversions of the nothing segfaults because r.Columns() on
a nil row segfaults.

To try to test this better, I added a filter to the executor test that
we use for this case, which got me a different result complaining about
a DistinctTimestamp result not being a SignedRow.

So, there's a couple of issues. One is that, in the case where a filter
is present, if the filter comes up with nothing, we can bail early
and return a result of the SignedRow type, which then breaks the reduce
part of our map/reduce when we try to reduce DistinctTimestamp values
into a SignedRow. To fix this, we make sure that we return the expected
type even in the case where we're bailing early.

A simpler way to see the actual original bug is, rather than having
a filter, just have a shard that has a value in *some other field*
but not in the timestamp field. So we add that to the test, too.

But also, really, since this is a problem that's happened more than
once, I propose that we also just make nil rows allow you to request
their columns and get back nil, so things like this don't bite us as
much. This wouldn't be a sufficient fix for the filter case, and I
still have the short-circuit for the nil row case explicitly in this
particular case because relying on the nil behavior bugs me, but I
think it's safer to allow .Columns on nil rows.
This commit is contained in:
Seebs 2022-02-16 14:18:25 -06:00
parent 69a4a0d779
commit 2aa10670fb
3 changed files with 29 additions and 4 deletions

View file

@ -1529,6 +1529,8 @@ func (e *executor) executeDistinctShard(ctx context.Context, qcx *Qcx, index str
Index: index,
Field: fieldName,
}
} else if field.Options().Type == FieldTypeTimestamp {
result = DistinctTimestamp{Name: fieldName}
} else {
result = SignedRow{}
}
@ -1564,11 +1566,19 @@ func (e *executor) executeDistinctShard(ctx context.Context, qcx *Qcx, index str
if err != nil {
return nil, err
}
results := make([]string, len(r.Pos.Columns()))
for i, val := range r.Pos.Columns() {
// If we have a filter, or there's just no content for this shard, we
// can end up with empty results. Rather than trying to synthesize
// a result from this empty set, we just go ahead and use that.
if r.Pos == nil {
return result, nil
}
cols := r.Pos.Columns()
results := make([]string, len(cols))
for i, val := range cols {
results[i] = FormatTimestampNano(int64(val), bsig.Base, field.options.TimeUnit)
}
return DistinctTimestamp{Name: fieldName, Values: results}, nil
result = DistinctTimestamp{Name: fieldName, Values: results}
return result, nil
}
return executeDistinctShardBSI(ctx, qcx, idx, fieldName, shard, bsig, filterBitmap)
}

View file

@ -6789,13 +6789,16 @@ func variousQueriesCountDistinctTimestamp(t *testing.T, c *test.Cluster) {
// create an index and timestamp field
c.CreateField(t, index, pilosa.IndexOptions{TrackExistence: true}, field, pilosa.OptFieldTypeTimestamp(time.Unix(0, 0), "s"))
c.CreateField(t, index, pilosa.IndexOptions{TrackExistence: true}, "set")
// add some data
data := []string{"2010-01-02T12:32:00Z", "2010-04-20T12:32:00Z", "2011-04-20T12:59:00Z", "2011-04-20T12:40:00Z", "2011-04-20T12:32:00Z"}
for i, datum := range data {
c.Query(t, index, fmt.Sprintf("Set(%d, ts=\"%s\")", i*(1<<20), datum))
c.Query(t, index, fmt.Sprintf("Set(%d, ts=\"%s\")", i*ShardWidth, datum))
}
// set something in shard 8 so there's a shard present with no timestamp data
c.Query(t, index, fmt.Sprintf("Set(%d, set=0)", 8*ShardWidth))
// query the Count of Distinct vals in field ts
count := c.Query(t, index, "Count(Distinct(field=ts))").Results[0]
@ -6803,6 +6806,13 @@ func variousQueriesCountDistinctTimestamp(t *testing.T, c *test.Cluster) {
t.Fatalf("expected %v got %v", len(data), count)
}
// query the ones that are in or after 2011, expecting 3. this helps us
// hit an edge case that only happens if you have no data *because of
// a filter*.
count = c.Query(t, index, "Count(Distinct(Row(ts > \"2011-01-01T00:00:00Z\"), field=ts))").Results[0]
if count != uint64(3) {
t.Fatalf("expected %v got %v", 3, count)
}
}
// Ensure that a top-level, bare distinct on multiple nodes

5
row.go
View file

@ -463,6 +463,11 @@ func (r *Row) MarshalJSON() ([]byte, error) {
// Columns returns the columns in r as a slice of ints.
func (r *Row) Columns() []uint64 {
// We occasionally hit cases where we want to call Columns on something
// that might not exist, but a nil slice would be fine.
if r == nil {
return nil
}
a := make([]uint64, 0, r.Count())
for i := range r.segments {
a = append(a, r.segments[i].Columns()...)