scriptoriumadmin/featurebase
1
0
Fork 0
mirror of https://github.com/featurebasedb/featurebase.git synced 2026-09-11 15:21:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

allow IDs to be passed even when keys enabled

Browse source 
This change allows one to query Pilosa fields and indexes directly
with integer row and column ids even when key translation is
enabled. This was previously disallowed during query
translation... I'm not sure why, but it can be quite useful for
debugging and testing to be able to use IDs directly. I have a test in
go-pilosa which uses this functionality.

I also simplified a bunch of the test code which was of the form:
```
else {
    if blah {
    }
}
```

to be:

```
else if blah {
```

which I think is pretty harmless.

I also changed a snapshot log line that has been bugging me to be
Debug level so that it isn't generating lots of useless logs for long
running Pilosa instances.
This commit is contained in:
Matt Jaffee 2019-11-27 08:41:40 -06:00
parent 16d5db97a6
commit 81a4d32cdd
No known key found for this signature in database
GPG key ID: 08A3DFFF987B11BF
3 changed files with 68 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
executor.go
View file

@ -3170,9 +3170,10 @@ func (e *executor) translateCall(index string, idx *Index, c *pql.Call) error {
// Translate column key.
if idx.Keys() {
if c.Args[colKey] != nil && !isString(c.Args[colKey]) {
return errors.New("column value must be a string when index 'keys' option enabled")
}
if value := callArgString(c, colKey); value != "" {
if !isValidID(c.Args[colKey]) {
return errors.Errorf("column value must be a string or non-negative integer, but got: %v of %[1]T", c.Args[colKey])
}
} else if value := callArgString(c, colKey); value != "" {
id, err := idx.translateStore.TranslateKey(value)
if err != nil {
return err
@ -3216,9 +3217,11 @@ func (e *executor) translateCall(index string, idx *Index, c *pql.Call) error {
}
} else if field.keys() {
if c.Args[rowKey] != nil && !isString(c.Args[rowKey]) {
return errors.New("row value must be a string when field 'keys' option enabled")
}
if value := callArgString(c, rowKey); value != "" {
// allow passing row id directly (this can come in handy, but make sure it is a valid row id)
if !isValidID(c.Args[rowKey]) {
return errors.Errorf("row value must be a string or non-negative integer, but got: %v of %[1]T", c.Args[rowKey])
}
} else if value := callArgString(c, rowKey); value != "" {
id, err := field.translateStore.TranslateKey(value)
if err != nil {
return err
@ -3626,6 +3629,29 @@ func isString(v interface{}) bool {
return ok
}
// isValidID returns whether v can be interpreted as a valid row or
// column ID. In short, is v a non-negative integer? I think the int64
// and default cases are the only ones actually used since the PQL
// parser doesn't return any other integer types.
func isValidID(v interface{}) bool {
switch vt := v.(type) {
case uint, uint64, uint32, uint16, uint8:
return true
case int64:
return vt >= 0
case int:
return vt >= 0
case int32:
return vt >= 0
case int16:
return vt >= 0
case int8:
return vt >= 0
default:
return false
}
}
// groupByIterator contains several slices. Each slice contains a number of
// elements equal to the number of fields in the group by (the number of Rows
// calls).

53
executor_test.go
View file

@ -526,10 +526,8 @@ func TestExecutor_Execute_Set(t *testing.T) {
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(1, f=11)`}); err != nil {
t.Fatal(err)
} else {
if !res.Results[0].(bool) {
t.Fatalf("expected column changed")
}
} else if !res.Results[0].(bool) {
t.Fatalf("expected column changed")
}
if n := hldr.Row("i", "f", 11).Count(); n != 1 {
@ -537,10 +535,8 @@ func TestExecutor_Execute_Set(t *testing.T) {
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(1, f=11)`}); err != nil {
t.Fatal(err)
} else {
if res.Results[0].(bool) {
t.Fatalf("expected column unchanged")
}
} else if res.Results[0].(bool) {
t.Fatalf("expected column unchanged")
}
})
@ -591,10 +587,8 @@ func TestExecutor_Execute_Set(t *testing.T) {
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("foo", f=11)`}); err != nil {
t.Fatal(err)
} else {
if !res.Results[0].(bool) {
t.Fatalf("expected column changed")
}
} else if !res.Results[0].(bool) {
t.Fatalf("expected column changed")
}
if n := hldr.Row("i", "f", 11).Count(); n != 1 {
@ -602,10 +596,20 @@ func TestExecutor_Execute_Set(t *testing.T) {
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("foo", f=11)`}); err != nil {
t.Fatal(err)
} else {
if res.Results[0].(bool) {
t.Fatalf("expected column unchanged")
}
} else if res.Results[0].(bool) {
t.Fatalf("expected column unchanged")
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f=11)`}); err != nil {
t.Fatal(err)
} else if !res.Results[0].(bool) {
t.Fatalf("expected column changed with integer column key")
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f=11)`}); err != nil {
t.Fatal(err)
} else if res.Results[0].(bool) {
t.Fatalf("expected column unchanged with integer column key")
}
})
@ -617,9 +621,15 @@ func TestExecutor_Execute_Set(t *testing.T) {
t.Fatal(err)
}
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f=1)`}); err == nil || errors.Cause(err).Error() != `column value must be a string when index 'keys' option enabled` {
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2.1, f=1)`}); err == nil || strings.Contains(err.Error(), `column value must be a string or non-negative integer`) {
t.Fatal(err)
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f=1)`}); err != nil {
t.Fatal(err)
} else if !res.Results[0].(bool) {
t.Fatalf("expected column changed with integer column key")
}
})
t.Run("ErrInvalidRowValueType", func(t *testing.T) {
@ -627,9 +637,16 @@ func TestExecutor_Execute_Set(t *testing.T) {
if _, err := index.CreateField("f", pilosa.OptFieldTypeDefault(), pilosa.OptFieldKeys()); err != nil {
t.Fatal(err)
}
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "inokey", Query: `Set(2, f=1)`}); err == nil || errors.Cause(err).Error() != `row value must be a string when field 'keys' option enabled` {
if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "inokey", Query: `Set(2, f=1.2)`}); err == nil || !strings.Contains(err.Error(), "row value must be a string or non-negative integer") {
t.Fatal(err)
}
if res, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2, f=9)`}); err != nil {
t.Fatal(err)
} else if !res.Results[0].(bool) {
t.Fatalf("expected column changed with integer column key")
}
})
})
}

2
snapshotqueue.go
View file

@ -394,7 +394,7 @@ func (sq *prioritySnapshotQueue) scanHolderWorker(h *Holder, background chan sna
sq.logger.Printf("background scan: %d fragments needed snapshots\n", hits)
hits = 0
} else {
sq.logger.Printf("background scan: no fragments needed snapshots, waiting\n")
sq.logger.Debugf("background scan: no fragments needed snapshots, waiting\n")
// No reason to be active if we're not finding anything.
select {
case <-time.After(60 * time.Second):