diff --git a/boltdb/translate.go b/boltdb/translate.go index 31d7364f4..a607d494f 100644 --- a/boltdb/translate.go +++ b/boltdb/translate.go @@ -11,9 +11,11 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package boltdb import ( + "bytes" "context" "os" "path/filepath" @@ -45,6 +47,14 @@ func OpenTranslateStore(path, index, field string, partitionID, partitionN int) var _ pilosa.TranslateStore = &TranslateStore{} // TranslateStore is an on-disk storage engine for translating string-to-uint64 values. +// An empty string will be converted into the sentinel byte slice: +// var emptyKey = []byte{ +// 0x00, 0x00, 0x00, +// 0x4d, 0x54, 0x4d, 0x54, // MTMT +// 0x00, +// 0xc2, 0xa0, // NO-BREAK SPACE +// 0x00, +// } type TranslateStore struct { mu sync.RWMutex db *bolt.DB @@ -149,7 +159,7 @@ func (s *TranslateStore) Size() int64 { func (s *TranslateStore) TranslateKey(key string) (id uint64, _ error) { // Find id by key under read lock. if err := s.db.View(func(tx *bolt.Tx) error { - id = findIDByKey(tx.Bucket([]byte("keys")), key) + id, _ = findIDByKey(tx.Bucket([]byte("keys")), key) return nil }); err != nil { return 0, err @@ -165,14 +175,16 @@ func (s *TranslateStore) TranslateKey(key string) (id uint64, _ error) { var written bool if err := s.db.Update(func(tx *bolt.Tx) (err error) { bkt := tx.Bucket([]byte("keys")) - if id = findIDByKey(bkt, key); id != 0 { + + var boltKey []byte + if id, boltKey = findIDByKey(bkt, key); id != 0 { return nil } id = pilosa.GenerateNextPartitionedID(s.index, maxID(tx), s.partitionID, s.partitionN) - if err := bkt.Put([]byte(key), u64tob(id)); err != nil { + if err := bkt.Put(boltKey, u64tob(id)); err != nil { return err - } else if err := tx.Bucket([]byte("ids")).Put(u64tob(id), []byte(key)); err != nil { + } else if err := tx.Bucket([]byte("ids")).Put(u64tob(id), boltKey); err != nil { return err } written = true @@ -203,7 +215,7 @@ func (s *TranslateStore) TranslateKeys(keys []string) (ids []uint64, _ error) { if err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket([]byte("keys")) for i, key := range keys { - if id := findIDByKey(bkt, key); id != 0 { + if id, _ := findIDByKey(bkt, key); id != 0 { ids[i] = id found++ } @@ -228,14 +240,15 @@ func (s *TranslateStore) TranslateKeys(keys []string) (ids []uint64, _ error) { continue } - if ids[i] = findIDByKey(bkt, key); ids[i] != 0 { + var boltKey []byte + if ids[i], boltKey = findIDByKey(bkt, key); ids[i] != 0 { continue } ids[i] = pilosa.GenerateNextPartitionedID(s.index, maxID(tx), s.partitionID, s.partitionN) - if err := bkt.Put([]byte(key), u64tob(ids[i])); err != nil { + if err := bkt.Put(boltKey, u64tob(ids[i])); err != nil { return err - } else if err := tx.Bucket([]byte("ids")).Put(u64tob(ids[i]), []byte(key)); err != nil { + } else if err := tx.Bucket([]byte("ids")).Put(u64tob(ids[i]), boltKey); err != nil { return err } written = true @@ -297,7 +310,7 @@ func (s *TranslateStore) ForceSet(id uint64, key string) error { return nil } -// Reader returns a reader that streams the underlying data file. +// EntryReader returns a reader that streams the underlying data file. func (s *TranslateStore) EntryReader(ctx context.Context, offset uint64) (pilosa.TranslateEntryReader, error) { ctx, cancel := context.WithCancel(ctx) return &TranslateEntryReader{ctx: ctx, cancel: cancel, store: s, offset: offset}, nil @@ -404,13 +417,33 @@ func (r *TranslateEntryReader) ReadEntry(entry *pilosa.TranslateEntry) error { } } -func findIDByKey(bkt *bolt.Bucket, key string) uint64 { - if value := bkt.Get([]byte(key)); value != nil { - return btou64(value) +// emptyKey is a sentinel byte slice which stands for "" as a key. +var emptyKey = []byte{ + 0x00, 0x00, 0x00, + 0x4d, 0x54, 0x4d, 0x54, // MTMT + 0x00, + 0xc2, 0xa0, // NO-BREAK SPACE + 0x00, +} + +func findIDByKey(bkt *bolt.Bucket, key string) (uint64, []byte) { + var boltKey []byte + if key == "" { + boltKey = emptyKey + } else { + boltKey = []byte(key) } - return 0 + + if value := bkt.Get(boltKey); value != nil { + return btou64(value), boltKey + } + return 0, boltKey } func findKeyByID(bkt *bolt.Bucket, id uint64) string { - return string(bkt.Get(u64tob(id))) + boltKey := bkt.Get(u64tob(id)) + if bytes.Equal(boltKey, emptyKey) { + return "" + } + return string(boltKey) } diff --git a/boltdb/translate_test.go b/boltdb/translate_test.go index 219885324..d3fb1b6e8 100644 --- a/boltdb/translate_test.go +++ b/boltdb/translate_test.go @@ -97,6 +97,10 @@ func TestTranslateStore_TranslateID(t *testing.T) { if err != nil { t.Fatal(err) } + id3, err := s.TranslateKey("") + if err != nil { + t.Fatal(err) + } // Ensure IDs can be translated back to keys. if key, err := s.TranslateID(id1); err != nil { @@ -110,6 +114,13 @@ func TestTranslateStore_TranslateID(t *testing.T) { } else if got, want := key, "bar"; got != want { t.Fatalf("TranslateID()=%s, want %s", got, want) } + + if key, err := s.TranslateID(id3); err != nil { + t.Fatal(err) + } else if got, want := key, ""; got != want { + t.Fatalf("TranslateID()=%s, want %s", got, want) + } + } func TestTranslateStore_TranslateIDs(t *testing.T) { diff --git a/cache.go b/cache.go index 6ff6f954a..9c0a7dc79 100644 --- a/cache.go +++ b/cache.go @@ -319,7 +319,7 @@ func (p bitmapPairs) Less(i, j int) bool { return p[i].Count > p[j].Count } // Pair holds an id/count pair. type Pair struct { ID uint64 `json:"id"` - Key string `json:"key,omitempty"` + Key string `json:"key"` Count uint64 `json:"count"` } diff --git a/docs/data-model.md b/docs/data-model.md index 7de27b575..85104bad3 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -121,6 +121,18 @@ Upon creation, fields are configured to be of a certain type. Pilosa supports th #### Set Set is the default field type in Pilosa. Set fields represent a standard, binary matrix of rows and columns where each row key represents a possible field value. The following example creates a `set` field called "info" with a ranked cache containing up to 100,000 records. +Row and/or column key can be a string literal (e.g. "value"). This mapping is also stored in a separate BoltDB data structure. Becauase BoltDB does not allow to have empty strings as keys, in pilosa we translate an empty string key into sentinel byte slice: +```go +[]byte{ + 0x00, 0x00, 0x00, + 0x4d, 0x54, 0x4d, 0x54, // MTMT + 0x00, + 0xc2, 0xa0, // NO-BREAK SPACE + 0x00, +} +``` +(where the first three bytes are _zero_ bytes, next four bytes stands for `MTMT` literal and the rest four bytes represent NBSP prefixed and suffixed with _zero_ byte). +In reverse translation, if we get from BoltDB the sentinel key, pilosa will rewrite it into an empty string (`""`). ``` request curl localhost:10101/index/repository/field/info \ diff --git a/executor.go b/executor.go index d0c53d43b..162680a24 100644 --- a/executor.go +++ b/executor.go @@ -2679,13 +2679,6 @@ func (e *executor) executeClearBit(ctx context.Context, index string, c *pql.Cal return false, ErrFieldNotFound } - // Clear column on existence field - if ef := idx.existenceField(); ef != nil { - if _, err := ef.ClearBit(0, colID); err != nil { - return false, errors.Wrap(err, "clearing existence column") - } - } - // Int field. if f.Type() == FieldTypeInt || f.Type() == FieldTypeDecimal { return e.executeClearValueField(ctx, index, c, f, colID, opt) diff --git a/executor_test.go b/executor_test.go index 5e969a977..e11429d0f 100644 --- a/executor_test.go +++ b/executor_test.go @@ -692,6 +692,48 @@ func TestExecutor_Execute_Clear(t *testing.T) { t.Fatalf("expected column changed") } }) + + t.Run("RowKeyColumnKey_NotClearNot", func(t *testing.T) { + writeQuery := `Set("056009039|q2db_3385|11", f="all_users")` + readQueries := []string{ + `Not(Row(f="has_deleted_date"))`, + `Clear("056009039|q2db_3385|11", f="has_deleted_date")`, + `Not(Row(f="has_deleted_date")) `, + } + results := []interface{}{ + "056009039|q2db_3385|11", + false, + "056009039|q2db_3385|11", + } + + responses := runCallTest(t, writeQuery, readQueries, &pilosa.IndexOptions{ + Keys: true, + TrackExistence: true, + }, pilosa.OptFieldKeys()) + for i, resp := range responses { + if len(resp.Results) != 1 { + t.Fatalf("response %d: len(results) expected: 1, got: %d", i, len(resp.Results)) + } + + switch r := resp.Results[0].(type) { + case bool: + if results[i] != r { + t.Fatalf("response %d: expected: %v, got: %v", i, results[i], r) + } + + case *pilosa.Row: + if len(r.Keys) != 1 { + t.Fatalf("response %d: len(keys) expected: 1, got: %d", i, len(r.Keys)) + } + if results[i] != r.Keys[0] { + t.Fatalf("response %d: expected: %v, got: %v", i, results[i], r.Keys[0]) + } + + default: + t.Fatalf("response %d: expected: %T, got: %T", i, results[i], r) + } + } + }) } // Ensure a set query can be executed on a bool field. diff --git a/field.go b/field.go index 618a3815d..e040b4441 100644 --- a/field.go +++ b/field.go @@ -1709,6 +1709,30 @@ type FieldOptions struct { ForeignIndex string `json:"foreignIndex"` } +// newFieldOptions returns a new instance of FieldOptions +// with applied and validated functional options. +func newFieldOptions(opts ...FieldOption) (*FieldOptions, error) { + fo := FieldOptions{} + for _, opt := range opts { + err := opt(&fo) + if err != nil { + return nil, err + } + } + + if fo.Keys { + switch fo.Type { + case FieldTypeInt: + return nil, ErrIntFieldWithKeys + + case FieldTypeDecimal: + return nil, ErrDecimalFieldWithKeys + } + } + + return &fo, nil +} + // applyDefaultOptions updates FieldOptions with the default // values if o does not contain a valid type. func applyDefaultOptions(o *FieldOptions) *FieldOptions { diff --git a/holder.go b/holder.go index 4fa554f1a..e7577f8ed 100644 --- a/holder.go +++ b/holder.go @@ -392,7 +392,7 @@ func (h *Holder) applySchema(schema *Schema) error { } // Create fields that don't exist. for _, f := range index.Fields { - field, err := idx.createFieldIfNotExists(f.Name, f.Options) + field, err := idx.createFieldIfNotExists(f.Name, &f.Options) if err != nil { return errors.Wrap(err, "creating field") } diff --git a/index.go b/index.go index 51e062823..b3d97a61f 100644 --- a/index.go +++ b/index.go @@ -250,7 +250,7 @@ fileLoop: // openExistenceField gets or creates the existence field and associates it to the index. func (i *Index) openExistenceField() error { - f, err := i.createFieldIfNotExists(existenceFieldName, FieldOptions{CacheType: CacheTypeNone, CacheSize: 0}) + f, err := i.createFieldIfNotExists(existenceFieldName, &FieldOptions{CacheType: CacheTypeNone, CacheSize: 0}) if err != nil { return errors.Wrap(err, "creating existence field") } @@ -401,13 +401,10 @@ func (i *Index) CreateField(name string, opts ...FieldOption) (*Field, error) { return nil, newConflictError(ErrFieldExists) } - // Apply functional options. - fo := FieldOptions{} - for _, opt := range opts { - err := opt(&fo) - if err != nil { - return nil, errors.Wrap(err, "applying option") - } + // Apply and validate functional options. + fo, err := newFieldOptions(opts...) + if err != nil { + return nil, errors.Wrap(err, "applying option") } return i.createField(name, fo) @@ -428,19 +425,16 @@ func (i *Index) CreateFieldIfNotExists(name string, opts ...FieldOption) (*Field return f, nil } - // Apply functional options. - fo := FieldOptions{} - for _, opt := range opts { - err := opt(&fo) - if err != nil { - return nil, errors.Wrap(err, "applying option") - } + // Apply and validate functional options. + fo, err := newFieldOptions(opts...) + if err != nil { + return nil, errors.Wrap(err, "applying option") } return i.createField(name, fo) } -func (i *Index) createFieldIfNotExists(name string, opt FieldOptions) (*Field, error) { +func (i *Index) createFieldIfNotExists(name string, opt *FieldOptions) (*Field, error) { i.mu.Lock() defer i.mu.Unlock() @@ -452,7 +446,7 @@ func (i *Index) createFieldIfNotExists(name string, opt FieldOptions) (*Field, e return i.createField(name, opt) } -func (i *Index) createField(name string, opt FieldOptions) (*Field, error) { +func (i *Index) createField(name string, opt *FieldOptions) (*Field, error) { if name == "" { return nil, errors.New("field name required") } else if opt.CacheType != "" && !isValidCacheType(opt.CacheType) { @@ -469,7 +463,7 @@ func (i *Index) createField(name string, opt FieldOptions) (*Field, error) { // up a foreign index. f.holder = i.holder - f.setOptions(&opt) + f.setOptions(opt) // Open field. if err := f.Open(); err != nil { diff --git a/index_test.go b/index_test.go index 77736331c..6a5811c03 100644 --- a/index_test.go +++ b/index_test.go @@ -185,6 +185,30 @@ func TestIndex_CreateField(t *testing.T) { }) */ }) + + t.Run("WithKeys", func(t *testing.T) { + // Don't allow an int field to be created with keys=true + t.Run("IntField", func(t *testing.T) { + index := test.MustOpenIndex() + defer index.Close() + + _, err := index.CreateField("f", pilosa.OptFieldTypeInt(-1, 1), pilosa.OptFieldKeys()) + if errors.Cause(err) != pilosa.ErrIntFieldWithKeys { + t.Fatal("int field cannot be created with keys=true") + } + }) + + // Don't allow a decimal field to be created with keys=true + t.Run("DecimalField", func(t *testing.T) { + index := test.MustOpenIndex() + defer index.Close() + + _, err := index.CreateField("f", pilosa.OptFieldTypeDecimal(1, -1, 1), pilosa.OptFieldKeys()) + if errors.Cause(err) != pilosa.ErrDecimalFieldWithKeys { + t.Fatal("decimal field cannot be created with keys=true") + } + }) + }) } // Ensure index can delete a field. diff --git a/pilosa.go b/pilosa.go index 3f1e8545b..369b72998 100644 --- a/pilosa.go +++ b/pilosa.go @@ -71,6 +71,9 @@ var ( ErrNotImplemented = errors.New("not implemented") ErrFieldsArgumentRequired = errors.New("fields argument required") ErrExpectedFieldListArgument = errors.New("expected field list argument") + + ErrIntFieldWithKeys = errors.New("int field cannot be created with 'keys=true' option") + ErrDecimalFieldWithKeys = errors.New("decimal field cannot be created with 'keys=true' option") ) // apiMethodNotAllowedError wraps an error value indicating that a particular diff --git a/server.go b/server.go index 19e20b074..455e4dd59 100644 --- a/server.go +++ b/server.go @@ -682,7 +682,7 @@ func (s *Server) receiveMessage(m Message) error { return fmt.Errorf("local index not found: %s", obj.Index) } opt := obj.Meta - _, err := idx.createFieldIfNotExists(obj.Field, *opt) + _, err := idx.createFieldIfNotExists(obj.Field, opt) if err != nil { return err } diff --git a/server/handler_test.go b/server/handler_test.go index 50aef2764..a24373ba1 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -492,7 +492,7 @@ func TestHandler_Endpoints(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/i0/query", strings.NewReader(`TopN(f0, n=2)`))) if w.Code != gohttp.StatusOK { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"results":[[{"id":30,"count":3},{"id":31,"count":1}]]}`+"\n" { + } else if body := w.Body.String(); body != `{"results":[[{"id":30,"key":"","count":3},{"id":31,"key":"","count":1}]]}`+"\n" { t.Fatalf("unexpected body: %q", body) } }) diff --git a/server/server_test.go b/server/server_test.go index fe8997c78..b3c82a142 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -371,7 +371,7 @@ func TestMain_RecalculateHashes(t *testing.T) { t.Fatalf("recalculating caches: %v", err) } - target := `{"results":[[{"id":7,"count":99},{"id":1,"count":99},{"id":9,"count":99},{"id":5,"count":99},{"id":4,"count":99},{"id":8,"count":99},{"id":2,"count":99},{"id":6,"count":99},{"id":3,"count":99}]]}` + target := `{"results":[[{"id":7,"key":"","count":99},{"id":1,"key":"","count":99},{"id":9,"key":"","count":99},{"id":5,"key":"","count":99},{"id":4,"key":"","count":99},{"id":8,"key":"","count":99},{"id":2,"key":"","count":99},{"id":6,"key":"","count":99},{"id":3,"key":"","count":99}]]}` // Run a TopN query on all nodes. The result should be the same as the target. for _, m := range cluster { diff --git a/translate.go b/translate.go index faf456054..b5f469f2f 100644 --- a/translate.go +++ b/translate.go @@ -39,6 +39,14 @@ var ( ) // TranslateStore is the storage for translation string-to-uint64 values. +// For BoltDB implementation an empty string will be converted into the sentinel byte slice: +// var emptyKey = []byte{ +// 0x00, 0x00, 0x00, +// 0x4d, 0x54, 0x4d, 0x54, // MTMT +// 0x00, +// 0xc2, 0xa0, // NO-BREAK SPACE +// 0x00, +// } type TranslateStore interface { io.Closer