Merge pull request #90 from travisturner/translation-sharding

Field.ForeignIndex translation on ImportValue()

api.go

@@ -1089,12 +1089,14 @@ func (api *API) ImportValue(ctx context.Context, req *ImportValueRequest, opts .
 		// Translate values when the field uses keys (for example, when
 		// the field has a ForeignIndex with keys).
 		if field.Keys() {
-			uints, err := field.translateStore.TranslateKeys(req.StringValues)
+			// Perform translation.
+			uints, err := api.cluster.translateIndexKeys(ctx, field.ForeignIndex(), req.StringValues)
 			if err != nil {
-				return errors.Wrap(err, "translating string values")
+				return err
 			}
-			// Because the BSI field supports negative value, we have to
-			// convert the slice of uint64 keys to a slice of int64.
+
+			// Because the BSI field supports negative values, we have to
+			// convert the uint64 keys to a slice of int64.
 			ints := make([]int64, len(uints))
 			for i := range uints {
 				ints[i] = int64(uints[i])

executor.go

@@ -3544,7 +3544,7 @@ func (e *executor) translateCalls(ctx context.Context, defaultIndexName string,
 	for indexName, keySet := range keySets {
 		idx := e.Holder.indexes[indexName]
 		if idx == nil {
-			return fmt.Errorf("canot find index %q", indexName)
+			return fmt.Errorf("cannot find index %q", indexName)
 		}
 
 		if !idx.Keys() || len(keySets) == 0 {
@@ -3656,17 +3656,6 @@ func (e *executor) translateCall(indexName string, c *pql.Call, keyMaps map[stri
 			return nil
 		}
 
-		// 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 errors.Errorf("foreign index not found: %q", foreignIndexName)
-			}
-			useKeys = foreignIndex.Keys()
-		}
-
 		// Bool field keys do not use the translator because there
 		// are only two possible values. Instead, they are handled
 		// directly.
@@ -3685,7 +3674,8 @@ func (e *executor) translateCall(indexName string, c *pql.Call, keyMaps map[stri
 				}
 				c.Args[rowKey] = rowID
 			}
-		} else if useKeys {
+		} else if field.Keys() {
+			foreignIndexName := field.ForeignIndex()
 			if c.Args[rowKey] != nil && isCondition(c.Args[rowKey]) {
 				// In the case where a field has a foreign index with keys,
 				// allow `== "key"` or `!= "key"` to be used against the BSI
@@ -3857,34 +3847,36 @@ 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 field.Keys() {
+				rslt := result.Pos
+				other := &Row{Attrs: rslt.Attrs}
+				for _, segment := range rslt.Segments() {
+					keys, err := e.Cluster.translateIndexIDs(context.Background(), field.ForeignIndex(), segment.Columns())
 					if err != nil {
-						return nil, err
+						return nil, errors.Wrap(err, "translating index ids")
 					}
-					other.Keys = append(other.Keys, key)
+					other.Keys = append(other.Keys, keys...)
 				}
+				return &SignedRow{Pos: other}, nil
 			}
-			return SignedRow{Pos: other}, nil
+
+			return nil, nil
+		}()
+		if err != nil {
+			return nil, err
+		} else if sr != nil {
+			return *sr, nil
 		}
 
 	case PairField:

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,31 @@ 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)
+		}
+	}
+	return len(diff) == 0
+}
+
 func TestExecutor_Execute_GroupBy(t *testing.T) {
 	groupByTest := func(t *testing.T, clusterSize int) {
 		c := test.MustRunCluster(t, 1)

field.go

@@ -520,12 +520,19 @@ func (f *Field) Open() error {
 			return errors.Wrap(err, "opening attrstore")
 		}
 
-		// If the field has a foreign index, and that index uses keys,
-		// then use that index's translateStore instead.
+		// Apply the field-specific translateStore.
 		if err := f.applyTranslateStore(); err != nil {
 			return errors.Wrap(err, "applying translate store")
 		}
 
+		// If the field has a foreign index, make sure the index
+		// exists.
+		if f.options.ForeignIndex != "" {
+			if err := f.holder.checkForeignIndex(f); err != nil {
+				return errors.Wrap(err, "checking foreign index")
+			}
+		}
+
 		return nil
 	}(); err != nil {
 		f.Close()
@@ -545,9 +552,25 @@ func (f *Field) applyTranslateStore() error {
 		return errors.Wrap(err, "opening field translate store")
 	}
 	f.usesKeys = f.options.Keys
+
+	// In the case where the field has a foreign index, set
+	// the usesKeys value accordingly.
+	if foreignIndexName := f.ForeignIndex(); foreignIndexName != "" {
+		if foreignIndex := f.holder.indexes[foreignIndexName]; foreignIndex != nil {
+			f.usesKeys = foreignIndex.Keys()
+		}
+	}
 	return nil
 }
 
+// applyForeignIndex used to set the field's translateStore to
+// that of the foreign index, but since moving to partitioned
+// translate stores on indexes, that doesn't happen anymore.
+// So now all this method does is check that the foreign index
+// actually exists. If we decided this was unnecessary (which
+// it kind of is), we could remove the field.holder and all
+// the logic which does this check on holder open after all
+// indexes have opened.
 func (f *Field) applyForeignIndex() error {
 	foreignIndex := f.holder.Index(f.options.ForeignIndex)
 	if foreignIndex == nil {