handle translation errors when using keys against an unkeyed index

This also adds tests for our error outputs.
This commit is contained in:
Nia Weiss 2021-03-17 11:57:06 -04:00
parent 16245b1742
commit 6f421dad69
No known key found for this signature in database
GPG key ID: 895E83409BFDA1BB
3 changed files with 72 additions and 19 deletions

View file

@ -1356,9 +1356,8 @@ func (c *cluster) findFieldKeys(ctx context.Context, field *Field, keys ...strin
// Therefore, the field keys are actually column keys on a different index.
return c.findIndexKeys(ctx, idx, keys...)
}
if !field.Keys() {
return nil, errors.Wrap(ErrTranslatingKeyNotFound, "field is not keyed")
return nil, errors.Errorf("cannot find keys on unkeyed field %q", field.Name())
}
// Attempt to find the keys locally.
@ -1421,7 +1420,7 @@ func (c *cluster) createFieldKeys(ctx context.Context, field *Field, keys ...str
}
if !field.Keys() {
return nil, errors.Wrap(ErrTranslatingKeyNotFound, "field is not keyed")
return nil, errors.Errorf("cannot create keys on unkeyed field %q", field.Name())
}
// The primary is the only node that can create field keys, since it owns the authoritative copy.
@ -1621,6 +1620,9 @@ func (c *cluster) findIndexKeys(ctx context.Context, indexName string, keys ...s
if idx == nil {
return nil, ErrIndexNotFound
}
if !idx.Keys() {
return nil, errors.Errorf("cannot find keys on unkeyed index %q", indexName)
}
// Create a snapshot of the cluster to use for node/partition calculations.
snap := topology.NewClusterSnapshot(c.noder, c.Hasher, c.ReplicaN)
@ -1728,7 +1730,7 @@ func (c *cluster) createIndexKeys(ctx context.Context, indexName string, keys ..
}
if !idx.keys {
return nil, errors.Errorf("can't create index keys on unkeyed index %s", indexName)
return nil, errors.Errorf("cannot create keys on unkeyed index %q", indexName)
}
// Create a snapshot of the cluster to use for node/partition calculations.

View file

@ -587,7 +587,7 @@ func TestExecutor_Execute_Set(t *testing.T) {
})
t.Run("ErrInvalidRowValueType", func(t *testing.T) {
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f="bar")`}); err == nil || !hasCause(err, pilosa.ErrTranslatingKeyNotFound) || !strings.Contains(err.Error(), "field is not keyed") {
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f="bar")`}); err == nil || !strings.Contains(err.Error(), "cannot create keys on unkeyed field") {
t.Fatal(err)
}
})
@ -990,26 +990,13 @@ func TestExecutor_Execute_SetValue(t *testing.T) {
})
t.Run("InvalidBSIGroupValueType", func(t *testing.T) {
if _, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(10, f="hello")`}); err == nil || !hasCause(err, pilosa.ErrTranslatingKeyNotFound) || !strings.Contains(err.Error(), "field is not keyed") {
if _, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(10, f="hello")`}); err == nil || !strings.Contains(err.Error(), "cannot create keys on unkeyed field") {
t.Fatalf("unexpected error: %s", err)
}
})
})
}
func hasCause(err, cause error) bool {
for err != cause {
innerErr := errors.Cause(err)
if innerErr == err {
// This is the innermost accessible error, and it does not have that cause.
return false
}
err = innerErr
}
return true
}
// Ensure a SetRowAttrs() query can be executed.
func TestExecutor_Execute_SetRowAttrs(t *testing.T) {
c := test.MustRunCluster(t, 1)

View file

@ -879,6 +879,70 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) {
})
}
func TestTranslation_Cluster_CreateFindUnkeyed(t *testing.T) {
c := test.MustRunCluster(t, 3)
defer c.Close()
c.CreateField(t, "i", pilosa.IndexOptions{}, "f")
t.Run("Index", func(t *testing.T) {
t.Run("Create", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
_, err := c.GetNonPrimary().API.CreateIndexKeys(ctx, "i", "foo")
if err == nil {
t.Fatal("unexpected success")
}
expect := `cannot create keys on unkeyed index "i"`
if got := err.Error(); got != expect {
t.Fatalf("expected error %q but got %q", expect, got)
}
})
t.Run("Find", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
_, err := c.GetNonPrimary().API.FindIndexKeys(ctx, "i", "foo")
if err == nil {
t.Fatal("unexpected success")
}
expect := `cannot find keys on unkeyed index "i"`
if got := err.Error(); got != expect {
t.Fatalf("expected error %q but got %q", expect, got)
}
})
})
t.Run("Field", func(t *testing.T) {
t.Run("Create", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
_, err := c.GetNonPrimary().API.CreateFieldKeys(ctx, "i", "f", "foo")
if err == nil {
t.Fatal("unexpected success")
}
expect := `cannot create keys on unkeyed field "f"`
if got := err.Error(); got != expect {
t.Fatalf("expected error %q but got %q", expect, got)
}
})
t.Run("Find", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
_, err := c.GetNonPrimary().API.FindFieldKeys(ctx, "i", "f", "foo")
if err == nil {
t.Fatal("unexpected success")
}
expect := `cannot find keys on unkeyed field "f"`
if got := err.Error(); got != expect {
t.Fatalf("expected error %q but got %q", expect, got)
}
})
})
}
func compareTranslations(expected, got map[string]uint64) error {
for key, id := range got {
if realID, ok := expected[key]; !ok {