From e40400b130f506de00b7ad27ccb3802320ecc61c Mon Sep 17 00:00:00 2001 From: Travis Date: Tue, 28 Jan 2020 22:41:02 -0600 Subject: [PATCH 1/2] Ensure ForeignIndex key translation happens in API. For Fields with ForeignIndex (which have keys), the API was missing the logic to do that translation against the translateStore of the foreign index. This commit adds that logic, as well as some missing translateStore-related logic in the gRPC code. --- api.go | 15 +++++++++++++ executor.go | 2 ++ field.go | 1 + server/grpc.go | 58 +++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/api.go b/api.go index ee24823a1..1ffbc7df6 100644 --- a/api.go +++ b/api.go @@ -545,6 +545,7 @@ func (api *API) ExportCSV(ctx context.Context, indexName string, fieldName strin var err error if field.Keys() { + // TODO: handle case: field.ForeignIndex if rowStr, err = field.TranslateStore().TranslateID(rowID); err != nil { return errors.Wrap(err, "translating row") } @@ -1451,6 +1452,10 @@ func (api *API) TranslateIndexKey(ctx context.Context, indexName string, key str return api.cluster.translateIndexKey(ctx, indexName, key) } +func (api *API) TranslateIndexIDs(ctx context.Context, indexName string, ids []uint64) ([]string, error) { + return api.cluster.translateIndexIDs(ctx, indexName, ids) +} + // TranslateKeys handles a TranslateKeyRequest. func (api *API) TranslateKeys(ctx context.Context, r io.Reader) (_ []byte, err error) { var req TranslateKeysRequest @@ -1469,6 +1474,11 @@ func (api *API) TranslateKeys(ctx context.Context, r io.Reader) (_ []byte, err e } else { if field := api.holder.Field(req.Index, req.Field); field == nil { return nil, ErrFieldNotFound + } else if fi := field.ForeignIndex(); fi != "" { + ids, err = api.cluster.translateIndexKeys(ctx, fi, req.Keys) + if err != nil { + return nil, err + } } else if ids, err = field.TranslateStore().TranslateKeys(req.Keys); err != nil { return nil, err } @@ -1500,6 +1510,11 @@ func (api *API) TranslateIDs(ctx context.Context, r io.Reader) (_ []byte, err er } else { if field := api.holder.Field(req.Index, req.Field); field == nil { return nil, ErrFieldNotFound + } else if fi := field.ForeignIndex(); fi != "" { + keys, err = api.cluster.translateIndexIDs(ctx, fi, req.IDs) + if err != nil { + return nil, err + } } else if keys, err = field.TranslateStore().TranslateIDs(req.IDs); err != nil { return nil, err } diff --git a/executor.go b/executor.go index d2c23da46..c078d4e29 100644 --- a/executor.go +++ b/executor.go @@ -3771,6 +3771,7 @@ func (e *executor) translateCall(indexName string, c *pql.Call, keyMaps map[stri if !ok { return errors.New("prev value must be a string when field 'keys' option enabled") } + // TODO: does this need to take field.ForeignIndex() into consideration? id, err := field.TranslateStore().TranslateKey(prevStr) if err != nil { return errors.Wrapf(err, "translating row key '%s'", prevStr) @@ -3937,6 +3938,7 @@ func (e *executor) translateResult(index string, idx *Index, call *pql.Call, res return nil, ErrFieldNotFound } if field.Keys() { + // TODO: does this need to take field.ForeignIndex() into consideration? key, err := field.TranslateStore().TranslateID(g.RowID) if err != nil { return nil, errors.Wrap(err, "translating row ID in Group") diff --git a/field.go b/field.go index 32dc18c22..618a3815d 100644 --- a/field.go +++ b/field.go @@ -576,6 +576,7 @@ func (f *Field) applyForeignIndex() error { if foreignIndex == nil { return errors.Wrapf(ErrForeignIndexNotFound, "%s", f.options.ForeignIndex) } + f.usesKeys = foreignIndex.Keys() return nil } diff --git a/server/grpc.go b/server/grpc.go index 4d410546f..95824c607 100644 --- a/server/grpc.go +++ b/server/grpc.go @@ -256,10 +256,31 @@ func (h grpcHandler) Inspect(req *pb.InspectRequest, stream pb.Pilosa_InspectSer case "int": if field.Keys() { - value, exists, err := field.StringValue(col) - if err != nil { - return errors.Wrap(err, "getting string field value for column") - } else if exists { + var value string + var exists bool + var err error + if fi := field.ForeignIndex(); fi != "" { + // Get the value from the int field. + intVal, ok, err := field.Value(col) + if err != nil { + return errors.Wrap(err, "getting int value") + } else if ok { + vals, err := h.api.TranslateIndexIDs(context.Background(), fi, []uint64{uint64(intVal)}) + if err != nil { + return errors.Wrap(err, "getting keys for ids") + } + if len(vals) > 0 && vals[0] != "" { + value = vals[0] + exists = true + } + } + } else { + value, exists, err = field.StringValue(col) + if err != nil { + return errors.Wrap(err, "getting string field value for column") + } + } + if exists { rowResp.Columns = append(rowResp.Columns, &pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_StringVal{StringVal: value}}) } else { @@ -460,10 +481,31 @@ func (h grpcHandler) Inspect(req *pb.InspectRequest, stream pb.Pilosa_InspectSer } if field.Keys() { - value, exists, err := field.StringValue(id) - if err != nil { - return errors.Wrap(err, "getting string field value for column") - } else if exists { + var value string + var exists bool + var err error + if fi := field.ForeignIndex(); fi != "" { + // Get the value from the int field. + intVal, ok, err := field.Value(id) + if err != nil { + return errors.Wrap(err, "getting int value") + } else if ok { + vals, err := h.api.TranslateIndexIDs(context.Background(), fi, []uint64{uint64(intVal)}) + if err != nil { + return errors.Wrap(err, "getting keys for ids") + } + if len(vals) > 0 && vals[0] != "" { + value = vals[0] + exists = true + } + } + } else { + value, exists, err = field.StringValue(id) + if err != nil { + return errors.Wrap(err, "getting string field value for column") + } + } + if exists { rowResp.Columns = append(rowResp.Columns, &pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_StringVal{StringVal: value}}) } else { From 61e527251ab689c99bc1174874389e66fdc04771 Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 30 Jan 2020 10:56:03 -0600 Subject: [PATCH 2/2] fix some comments --- executor.go | 2 +- index.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/executor.go b/executor.go index c078d4e29..473df14b5 100644 --- a/executor.go +++ b/executor.go @@ -3583,7 +3583,7 @@ func (e *executor) collectCallKeySets(ctx context.Context, indexName string, c * } } - // Collection foreign index keys. + // Collect foreign index keys. if fieldName != "" { idx := e.Holder.indexes[indexName] if field := idx.Field(fieldName); field != nil && field.ForeignIndex() != "" { diff --git a/index.go b/index.go index 56cbc0de2..bc6293b1b 100644 --- a/index.go +++ b/index.go @@ -62,8 +62,7 @@ type Index struct { logger logger.Logger snapshotQueue snapshotQueue - // Used for notifying holder when a field is added. - // Also passed to field for foreign-index lookup. + // Passed to field for foreign-index lookup. holder *Holder // Per-partition translation stores