From 90a2e116a7cdfc9073eec7ede0467a60b7172d7c Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 17 Jan 2020 11:10:42 -0600 Subject: [PATCH] update translateResult to translate foreign index keys on SignedRow results --- executor.go | 57 +++++++++++++++++++++++++++++------------------- executor_test.go | 30 ++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/executor.go b/executor.go index b6120662e..e3a064754 100644 --- a/executor.go +++ b/executor.go @@ -3857,34 +3857,47 @@ func (e *executor) translateResult(index string, idx *Index, call *pql.Call, res // make the return type for an int field with a ForeignIndex be // a *Row instead (because it should always be positive). case SignedRow: - var store TranslateStore + sr, err := func() (*SignedRow, error) { + fieldName := callArgString(call, "field") + if fieldName == "" { + return nil, nil + } - if fieldName := callArgString(call, "field"); fieldName != "" { field := idx.Field(fieldName) - if field != nil && field.Keys() { - store = field.TranslateStore() + if field == nil { + return nil, nil } - } - // In the case where a field/foreignIndex doesn't exist, - // fall back to using the index translateStore. - if store == nil && idx.Keys() { - store = nil // TODO: this may need to be idx.TranslateStore(?) - } - - if store != nil { - rslt := result.Pos - other := &Row{Attrs: rslt.Attrs} - for _, segment := range rslt.Segments() { - for _, col := range segment.Columns() { - key, err := store.TranslateID(col) - if err != nil { - return nil, err - } - other.Keys = append(other.Keys, key) + // Determine if foreign index is being used for translation. + useKeys := field.Keys() + foreignIndexName := field.ForeignIndex() + if foreignIndexName != "" { + foreignIndex := e.Holder.indexes[foreignIndexName] + if foreignIndex == nil { + return nil, errors.Errorf("foreign index not found: %q", foreignIndexName) } + useKeys = foreignIndex.Keys() } - return SignedRow{Pos: other}, nil + + if useKeys { + rslt := result.Pos + other := &Row{Attrs: rslt.Attrs} + for _, segment := range rslt.Segments() { + keys, err := e.Cluster.translateIndexIDs(context.Background(), foreignIndexName, segment.Columns()) + if err != nil { + return nil, errors.Wrap(err, "translating index ids") + } + other.Keys = append(other.Keys, keys...) + } + return &SignedRow{Pos: other}, nil + } + + return nil, nil + }() + if err != nil { + return nil, err + } else if sr != nil { + return *sr, nil } case PairField: diff --git a/executor_test.go b/executor_test.go index afd322eb6..0ec94efe4 100644 --- a/executor_test.go +++ b/executor_test.go @@ -3898,7 +3898,7 @@ func TestExecutor_ForeignIndex(t *testing.T) { `) distinct := c.Query(t, "child", `Distinct(index="child", field="parent_id")`).Results[0].(pilosa.SignedRow) - if !reflect.DeepEqual(distinct.Pos.Keys, []string{"one", "two", "twenty-one"}) { + if !sameStringSlice(distinct.Pos.Keys, []string{"one", "two", "twenty-one"}) { t.Fatalf("unexpected keys: %v", distinct.Pos.Keys) } @@ -3918,6 +3918,34 @@ func TestExecutor_ForeignIndex(t *testing.T) { } } +// sameStringSlice is a helper function which compares two string +// slices without enforcing order. +func sameStringSlice(x, y []string) bool { + if len(x) != len(y) { + return false + } + // create a map of string -> int + diff := make(map[string]int, len(x)) + for _, _x := range x { + // 0 value for int is 0, so just increment a counter for the string + diff[_x]++ + } + for _, _y := range y { + // If the string _y is not in diff bail out early + if _, ok := diff[_y]; !ok { + return false + } + diff[_y] -= 1 + if diff[_y] == 0 { + delete(diff, _y) + } + } + if len(diff) == 0 { + return true + } + return false +} + func TestExecutor_Execute_GroupBy(t *testing.T) { groupByTest := func(t *testing.T, clusterSize int) { c := test.MustRunCluster(t, 1)