Skip to content

Commit

Permalink
[dbnode] Allow returning empty term values from aggregate index value…
Browse files Browse the repository at this point in the history
…s RPC (#2120)
  • Loading branch information
robskillington authored Jan 24, 2020
1 parent d141806 commit 829d13e
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 17 deletions.
4 changes: 0 additions & 4 deletions src/dbnode/storage/index/aggregate_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,6 @@ func (r *aggregatedResults) addFieldWithLock(
return fmt.Errorf(missingDocumentFields, "term")
}

if len(value) == 0 {
return fmt.Errorf(missingDocumentFields, "value")
}

// if a term filter is provided, ensure this field matches the filter,
// otherwise ignore it.
filter := r.aggregateOpts.FieldFilter
Expand Down
11 changes: 7 additions & 4 deletions src/dbnode/storage/index/aggregate_results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,14 @@ func TestAggResultsInsertInvalid(t *testing.T) {
size, err = res.AddDocuments([]doc.Document{dInvalid})
require.Error(t, err)
require.Equal(t, 0, size)
}

dInvalid = genDoc("foo", "")
size, err = res.AddDocuments([]doc.Document{dInvalid})
require.Error(t, err)
require.Equal(t, 0, size)
func TestAggResultsInsertEmptyTermValue(t *testing.T) {
res := NewAggregateResults(nil, AggregateResultsOptions{}, testOpts)
dValidEmptyTerm := genDoc("foo", "")
size, err := res.AddDocuments([]doc.Document{dValidEmptyTerm})
require.NoError(t, err)
require.Equal(t, 1, size)
}

func TestAggResultsTermOnlyInsert(t *testing.T) {
Expand Down
11 changes: 2 additions & 9 deletions src/dbnode/storage/index/aggregate_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,10 @@
package index

import (
"errors"

"github.com/m3db/m3/src/x/ident"
"github.com/m3db/m3/src/x/pool"
)

var (
errUnableToAddValueMissingID = errors.New("no id for value")
)

// AggregateValues is a collection of unique identity values backed by a pool.
// NB: there are no synchronization guarantees provided by default.
type AggregateValues struct {
Expand Down Expand Up @@ -83,10 +77,9 @@ func (v *AggregateValues) finalize() {
}

func (v *AggregateValues) addValue(value ident.ID) error {
// NB(r): Allow for empty values to be set, an empty string
// is still different from not having the field at all.
bytesID := ident.BytesID(value.Bytes())
if len(bytesID) == 0 {
return errUnableToAddValueMissingID
}

// NB: fine to overwrite the values here.
v.valuesMap.Set(bytesID, struct{}{})
Expand Down

0 comments on commit 829d13e

Please sign in to comment.