From 3781b6e7eb40352b7a2defe6a4574800cd1cc743 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 3 Jul 2018 11:05:33 -0500 Subject: [PATCH 1/5] remove redundant calls to SetupServer --- test/pilosa.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/pilosa.go b/test/pilosa.go index 4774b4cf8..a40f2e956 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -63,11 +63,6 @@ func NewCommand(opts ...server.CommandOption) *Command { m.Command.Stdout = &m.Stdout m.Command.Stderr = &m.Stderr - err = m.SetupServer() - if err != nil { - panic(err) - } - if testing.Verbose() { m.Command.Stdout = io.MultiWriter(os.Stdout, m.Command.Stdout) m.Command.Stderr = io.MultiWriter(os.Stderr, m.Command.Stderr) @@ -117,10 +112,6 @@ func (m *Command) Reopen() error { config := m.Command.Config m.Command = server.NewCommand(os.Stdin, os.Stdout, os.Stderr, m.commandOptions...) m.Command.Config = config - err := m.SetupServer() - if err != nil { - return errors.Wrap(err, "setting up server") - } // Run new program. if err := m.Start(); err != nil { From 1fb8330c8cf0d351c228a1345db19fa0506ee186 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 4 Jul 2018 12:27:42 -0500 Subject: [PATCH 2/5] remove some dead code highlighed by the unexport script --- api.go | 57 ++----------------------------------------- apimethod_string.go | 4 +-- cluster.go | 3 +-- field.go | 7 ------ row.go | 28 --------------------- time.go | 9 ------- time_internal_test.go | 16 +++++++++--- uri.go | 12 ++------- uri_internal_test.go | 10 -------- 9 files changed, 20 insertions(+), 126 deletions(-) diff --git a/api.go b/api.go index abf2a84ea..be75624bc 100644 --- a/api.go +++ b/api.go @@ -370,55 +370,6 @@ func (api *API) ShardNodes(ctx context.Context, indexName string, shard uint64) return api.cluster.shardNodes(indexName, shard), nil } -// MarshalFragment returns an object which can write the specified fragment's data -// to an io.Writer. The serialized data can be read back into a fragment with -// the UnmarshalFragment API call. -func (api *API) MarshalFragment(ctx context.Context, indexName string, fieldName string, shard uint64) (io.WriterTo, error) { - if err := api.validate(apiMarshalFragment); err != nil { - return nil, errors.Wrap(err, "validating api method") - } - - // Retrieve fragment from holder. - f := api.holder.fragment(indexName, fieldName, viewStandard, shard) - if f == nil { - return nil, ErrFragmentNotFound - } - return f, nil -} - -// UnmarshalFragment creates a new fragment (if necessary) and reads data from a -// Reader which was previously written by MarshalFragment to populate the -// fragment's data. -func (api *API) UnmarshalFragment(ctx context.Context, indexName string, fieldName string, shard uint64, reader io.ReadCloser) error { - if err := api.validate(apiUnmarshalFragment); err != nil { - return errors.Wrap(err, "validating api method") - } - - // Retrieve field. - f := api.holder.Field(indexName, fieldName) - if f == nil { - return ErrFieldNotFound - } - - // Retrieve view. - view, err := f.createViewIfNotExists(viewStandard) - if err != nil { - return errors.Wrap(err, "creating view") - } - - // Retrieve fragment from field. - frag, err := view.CreateFragmentIfNotExists(shard) - if err != nil { - return errors.Wrap(err, "creating fragment") - } - - // Read fragment in from request body. - if _, err := frag.ReadFrom(reader); err != nil { - return errors.Wrap(err, "reading fragment") - } - return nil -} - // FragmentBlockData is an endpoint for internal usage. It is not guaranteed to // return anything useful. Currently it returns protobuf encoded row and column // ids from a "block" which is a subdivision of a fragment. @@ -889,7 +840,6 @@ const ( apiIndexAttrDiff //apiLocalID // not implemented //apiLongQueryTime // not implemented - apiMarshalFragment //apiMaxShards // not implemented apiQuery apiRecalculateCaches @@ -900,15 +850,13 @@ const ( apiShardNodes //apiState // not implemented //apiStatsWithTags // not implemented - apiUnmarshalFragment //apiVersion // not implemented apiViews ) var methodsCommon = map[apiMethod]struct{}{ - apiClusterMessage: struct{}{}, - apiMarshalFragment: struct{}{}, - apiSetCoordinator: struct{}{}, + apiClusterMessage: struct{}{}, + apiSetCoordinator: struct{}{}, } var methodsResizing = map[apiMethod]struct{}{ @@ -934,6 +882,5 @@ var methodsNormal = map[apiMethod]struct{}{ apiRecalculateCaches: struct{}{}, apiRemoveNode: struct{}{}, apiShardNodes: struct{}{}, - apiUnmarshalFragment: struct{}{}, apiViews: struct{}{}, } diff --git a/apimethod_string.go b/apimethod_string.go index 881b79472..01217092f 100644 --- a/apimethod_string.go +++ b/apimethod_string.go @@ -4,9 +4,9 @@ package pilosa import "strconv" -const _apiMethod_name = "apiClusterMessageapiCreateFieldapiCreateIndexapiDeleteFieldapiDeleteIndexapiDeleteViewapiExportCSVapiFragmentBlockDataapiFragmentBlocksapiFieldapiFieldAttrDiffapiImportapiImportValueapiIndexapiIndexAttrDiffapiMarshalFragmentapiQueryapiRecalculateCachesapiRemoveNodeapiResizeAbortapiSetCoordinatorapiShardNodesapiUnmarshalFragmentapiViews" +const _apiMethod_name = "apiClusterMessageapiCreateFieldapiCreateIndexapiDeleteFieldapiDeleteIndexapiDeleteViewapiExportCSVapiFragmentBlockDataapiFragmentBlocksapiFieldapiFieldAttrDiffapiImportapiImportValueapiIndexapiIndexAttrDiffapiQueryapiRecalculateCachesapiRemoveNodeapiResizeAbortapiSetCoordinatorapiShardNodesapiViews" -var _apiMethod_index = [...]uint16{0, 17, 31, 45, 59, 73, 86, 98, 118, 135, 143, 159, 168, 182, 190, 206, 224, 232, 252, 265, 279, 296, 309, 329, 337} +var _apiMethod_index = [...]uint16{0, 17, 31, 45, 59, 73, 86, 98, 118, 135, 143, 159, 168, 182, 190, 206, 214, 234, 247, 261, 278, 291, 299} func (i apiMethod) String() string { if i < 0 || i >= apiMethod(len(_apiMethod_index)-1) { diff --git a/cluster.go b/cluster.go index acf06dae0..3fe0dbfa5 100644 --- a/cluster.go +++ b/cluster.go @@ -45,8 +45,7 @@ const ( ClusterStateResizing = "RESIZING" // NodeState represents the state of a node during startup. - NodeStateLoading = "LOADING" - NodeStateReady = "READY" + NodeStateReady = "READY" // resizeJob states. resizeJobStateRunning = "RUNNING" diff --git a/field.go b/field.go index 76be9cf3f..56faffcf1 100644 --- a/field.go +++ b/field.go @@ -178,13 +178,6 @@ func (f *Field) Type() string { return f.options.Type } -// CacheType returns the caching mode for the field. -func (f *Field) CacheType() string { - f.mu.RLock() - defer f.mu.RUnlock() - return f.options.CacheType -} - // SetCacheSize sets the cache size for ranked fames. Persists to meta file on update. // defaults to DefaultCacheSize 50000 func (f *Field) SetCacheSize(v uint32) error { diff --git a/row.go b/row.go index cbfa6b270..4c722c936 100644 --- a/row.go +++ b/row.go @@ -216,25 +216,6 @@ func (r *Row) InvalidateCount() { } } -// IncrementCount increments the row cached counter, note this is an optimization that assumes that the caller is aware the size increased. -func (r *Row) IncrementCount(i uint64) { - seg := r.segment(i / ShardWidth) - if seg != nil { - seg.n++ - } - -} - -// DecrementCount decrements the row cached counter. -func (r *Row) DecrementCount(i uint64) { - seg := r.segment(i / ShardWidth) - if seg != nil { - if seg.n > 0 { - seg.n-- - } - } -} - // Count returns the number of columns in the row. func (r *Row) Count() uint64 { var n uint64 @@ -297,15 +278,6 @@ func DecodeRow(pr *internal.Row) *Row { return r } -// Union performs a union on a slice of rows. -func Union(rows []*Row) *Row { - other := rows[0] - for _, r := range rows[1:] { - other = other.Union(r) - } - return other -} - // RowSegment holds a subset of a row. // This could point to a mmapped roaring bitmap or an in-memory bitmap. The // width of the segment will always match the shard width. diff --git a/time.go b/time.go index def889304..ecfdcad3d 100644 --- a/time.go +++ b/time.go @@ -70,15 +70,6 @@ func (q TimeQuantum) Type() string { return "TimeQuantum" } -// ParseTimeQuantum parses v into a time quantum. -func ParseTimeQuantum(v string) (TimeQuantum, error) { - q := TimeQuantum(strings.ToUpper(v)) - if !q.Valid() { - return "", ErrInvalidTimeQuantum - } - return q, nil -} - // viewByTimeUnit returns the view name for time with a given quantum unit. func viewByTimeUnit(name string, t time.Time, unit rune) string { switch unit { diff --git a/time_internal_test.go b/time_internal_test.go index 2920685fc..bd4afae43 100644 --- a/time_internal_test.go +++ b/time_internal_test.go @@ -16,6 +16,7 @@ package pilosa import ( "reflect" + "strings" "testing" "time" ) @@ -23,7 +24,7 @@ import ( // Ensure string can be parsed into time quantum. func TestParseTimeQuantum(t *testing.T) { t.Run("OK", func(t *testing.T) { - if q, err := ParseTimeQuantum("YMDH"); err != nil { + if q, err := parseTimeQuantum("YMDH"); err != nil { t.Fatalf("unexpected error: %s", err) } else if q != TimeQuantum("YMDH") { t.Fatalf("unexpected quantum: %#v", q) @@ -31,7 +32,7 @@ func TestParseTimeQuantum(t *testing.T) { }) t.Run("ErrInvalidTimeQuantum", func(t *testing.T) { - if _, err := ParseTimeQuantum("BADQUANTUM"); err != ErrInvalidTimeQuantum { + if _, err := parseTimeQuantum("BADQUANTUM"); err != ErrInvalidTimeQuantum { t.Fatalf("unexpected error: %s", err) } }) @@ -160,9 +161,18 @@ func mustParseTime(value string) time.Time { // mustParseTimeQuantum parses v into a time quantum. Panic on error. func mustParseTimeQuantum(v string) TimeQuantum { - q, err := ParseTimeQuantum(v) + q, err := parseTimeQuantum(v) if err != nil { panic(err) } return q } + +// parseTimeQuantum parses v into a time quantum. +func parseTimeQuantum(v string) (TimeQuantum, error) { + q := TimeQuantum(strings.ToUpper(v)) + if !q.Valid() { + return "", ErrInvalidTimeQuantum + } + return q, nil +} diff --git a/uri.go b/uri.go index 5d823d8b6..4eac6253e 100644 --- a/uri.go +++ b/uri.go @@ -148,14 +148,6 @@ func (u URI) String() string { return fmt.Sprintf("%s://%s:%d", u.scheme, u.host, u.port) } -// Equals returns true if the checked URI is equivalent to this URI. -func (u URI) Equals(other *URI) bool { - if other == nil { - return false - } - return u == *other -} - // Path returns URI with path func (u *URI) Path(path string) string { return fmt.Sprintf("%s%s", u.Normalize(), path) @@ -163,7 +155,7 @@ func (u *URI) Path(path string) string { // The following methods are required to implement pflag Value interface. -// Set sets the time quantum value. +// Set sets the uri value. func (u *URI) Set(value string) error { uri, err := NewURIFromAddress(value) if err != nil { @@ -173,7 +165,7 @@ func (u *URI) Set(value string) error { return nil } -// Type returns the type of a time quantum value. +// Type returns the type of a uri. func (u URI) Type() string { return "URI" } diff --git a/uri_internal_test.go b/uri_internal_test.go index dbcbfa04d..37dbddb70 100644 --- a/uri_internal_test.go +++ b/uri_internal_test.go @@ -76,16 +76,6 @@ func TestURIPath(t *testing.T) { } } -func TestEquals(t *testing.T) { - uri1 := DefaultURI() - if uri1.Equals(nil) { - t.Fatalf("URI should not be equal to nil") - } - if !uri1.Equals(DefaultURI()) { - t.Fatalf("URI should be equal to another URI with the same scheme, host and port") - } -} - func TestSetScheme(t *testing.T) { uri := DefaultURI() target := "fun" From 44f0b992f6170efc2d802be99942b1a71f25f8ee Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 4 Jul 2018 21:22:35 -0500 Subject: [PATCH 3/5] change all CreateField() methods to take functional options instead of FieldOptions --- api.go | 2 +- cluster_internal_test.go | 4 +- executor_test.go | 102 +++++++++++--------------------------- field.go | 30 ++++++++++- field_internal_test.go | 18 +++---- field_test.go | 34 +++---------- fragment_internal_test.go | 4 +- holder.go | 2 +- holder_internal_test.go | 8 +-- holder_test.go | 10 ++-- http/client_test.go | 7 +-- index.go | 37 ++++++++++++-- index_test.go | 17 ++----- server.go | 2 +- server/handler_test.go | 10 ++-- test/field.go | 12 ++--- test/holder.go | 12 ++--- test/index.go | 8 +-- utils_internal_test.go | 4 +- 19 files changed, 151 insertions(+), 172 deletions(-) diff --git a/api.go b/api.go index abf2a84ea..f0e2dbfb0 100644 --- a/api.go +++ b/api.go @@ -257,7 +257,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str } // Create field. - field, err := index.CreateField(fieldName, fo) + field, err := index.CreateField(fieldName, opts) if err != nil { return nil, errors.Wrap(err, "creating field") } diff --git a/cluster_internal_test.go b/cluster_internal_test.go index aee4ea09b..6dc79fdef 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -150,7 +150,7 @@ func TestFragSources(t *testing.T) { c5.addNodeBasicSorted(node3) idx := newIndexWithTempPath("i") - field, err := idx.CreateFieldIfNotExists("f", FieldOptions{}) + field, err := idx.CreateFieldIfNotExists("f", OptFieldTypeDefault()) if err != nil { t.Fatal(err) } @@ -697,7 +697,7 @@ func TestCluster_ResizeStates(t *testing.T) { } // Add Bit Data to node0. - if err := tc.CreateField("i", "f", FieldOptions{}); err != nil { + if err := tc.CreateField("i", "f", OptFieldTypeDefault()); err != nil { t.Fatal(err) } tc.SetBit("i", "f", 1, 101, nil) diff --git a/executor_test.go b/executor_test.go index f54fb7e33..c5e2f24a8 100644 --- a/executor_test.go +++ b/executor_test.go @@ -39,7 +39,7 @@ func TestExecutor_Execute_Bitmap(t *testing.T) { defer c.Close() hldr := test.Holder{Holder: c[0].Server.Holder()} index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - f, err := index.CreateField("f", pilosa.FieldOptions{}) + f, err := index.CreateField("f", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatal(err) } @@ -89,7 +89,7 @@ func TestExecutor_Execute_Bitmap(t *testing.T) { hldr := test.Holder{Holder: c[0].Server.Holder()} index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - if _, err := index.CreateField("f", pilosa.FieldOptions{}); err != nil { + if _, err := index.CreateField("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } @@ -112,7 +112,7 @@ func TestExecutor_Execute_Bitmap(t *testing.T) { hldr := test.Holder{Holder: c[0].Server.Holder()} index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{Keys: true}) - if _, err := index.CreateField("f", pilosa.FieldOptions{Keys: true}); err != nil { + if _, err := index.CreateField("f", pilosa.OptFieldTypeDefault(), pilosa.OptFieldKeys()); err != nil { t.Fatal(err) } @@ -354,7 +354,7 @@ func TestExecutor_Execute_SetBit(t *testing.T) { if err := index.DeleteField("f"); err != nil { t.Fatal(err) } - if _, err := index.CreateField("f", pilosa.FieldOptions{}); err != nil { + if _, err := index.CreateField("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } @@ -365,7 +365,7 @@ func TestExecutor_Execute_SetBit(t *testing.T) { t.Run("ErrInvalidRowValueType", func(t *testing.T) { index := hldr.MustCreateIndexIfNotExists("inokey", pilosa.IndexOptions{}) - if _, err := index.CreateField("f", pilosa.FieldOptions{Keys: true}); err != nil { + 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` { @@ -398,13 +398,9 @@ func TestExecutor_Execute_SetValue(t *testing.T) { // Create felds. index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 50, - }); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeInt(0, 50)); err != nil { t.Fatal(err) - } else if _, err := index.CreateFieldIfNotExists("xxx", pilosa.FieldOptions{}); err != nil { + } else if _, err := index.CreateFieldIfNotExists("xxx", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } @@ -439,11 +435,7 @@ func TestExecutor_Execute_SetValue(t *testing.T) { hldr := test.Holder{Holder: c[0].Server.Holder()} index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 100, - }); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeInt(0, 100)); err != nil { t.Fatal(err) } @@ -475,9 +467,9 @@ func TestExecutor_Execute_SetRowAttrs(t *testing.T) { // Create fields. index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{}); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) - } else if _, err := index.CreateFieldIfNotExists("xxx", pilosa.FieldOptions{}); err != nil { + } else if _, err := index.CreateFieldIfNotExists("xxx", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } @@ -514,9 +506,9 @@ func TestExecutor_Execute_TopN(t *testing.T) { // Set columns for rows 0, 10, & 20 across two shards. if idx, err := hldr.CreateIndex("i", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("f", pilosa.FieldOptions{}); err != nil { + } else if _, err := idx.CreateField("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("other", pilosa.FieldOptions{}); err != nil { + } else if _, err := idx.CreateField("other", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: ` Set(0, f=0) @@ -555,9 +547,9 @@ func TestExecutor_Execute_TopN(t *testing.T) { // Set columns for rows 0, 10, & 20 across two shards. if idx, err := hldr.CreateIndex("i", pilosa.IndexOptions{Keys: true}); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("f", pilosa.FieldOptions{Keys: true}); err != nil { + } else if _, err := idx.CreateField("f", pilosa.OptFieldTypeDefault(), pilosa.OptFieldKeys()); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("other", pilosa.FieldOptions{Keys: true}); err != nil { + } else if _, err := idx.CreateField("other", pilosa.OptFieldTypeDefault(), pilosa.OptFieldKeys()); err != nil { t.Fatal(err) } else if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: ` Set("a", f="foo") @@ -741,15 +733,11 @@ func TestExecutor_Execute_MinMax(t *testing.T) { t.Fatal(err) } - if _, err := idx.CreateField("x", pilosa.FieldOptions{}); err != nil { + if _, err := idx.CreateField("x", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: -10, - Max: 100, - }); err != nil { + if _, err := idx.CreateField("f", pilosa.OptFieldTypeInt(-10, 100)); err != nil { t.Fatal(err) } @@ -836,31 +824,19 @@ func TestExecutor_Execute_Sum(t *testing.T) { t.Fatal(err) } - if _, err := idx.CreateField("x", pilosa.FieldOptions{}); err != nil { + if _, err := idx.CreateField("x", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("foo", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 10, - Max: 100, - }); err != nil { + if _, err := idx.CreateField("foo", pilosa.OptFieldTypeInt(10, 100)); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("bar", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 100000, - }); err != nil { + if _, err := idx.CreateField("bar", pilosa.OptFieldTypeInt(0, 100000)); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("other", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 1000, - }); err != nil { + if _, err := idx.CreateField("other", pilosa.OptFieldTypeInt(0, 1000)); err != nil { t.Fatal(err) } @@ -906,10 +882,7 @@ func TestExecutor_Execute_Range(t *testing.T) { index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) // Create field. - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeTime, - TimeQuantum: pilosa.TimeQuantum("YMDH"), - }); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeTime(pilosa.TimeQuantum("YMDH"))); err != nil { t.Fatal(err) } @@ -962,39 +935,23 @@ func TestExecutor_Execute_BSIGroupRange(t *testing.T) { t.Fatal(err) } - if _, err := idx.CreateField("f", pilosa.FieldOptions{}); err != nil { + if _, err := idx.CreateField("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("foo", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 10, - Max: 100, - }); err != nil { + if _, err := idx.CreateField("foo", pilosa.OptFieldTypeInt(10, 100)); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("bar", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 100000, - }); err != nil { + if _, err := idx.CreateField("bar", pilosa.OptFieldTypeInt(0, 100000)); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("other", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 1000, - }); err != nil { + if _, err := idx.CreateField("other", pilosa.OptFieldTypeInt(0, 1000)); err != nil { t.Fatal(err) } - if _, err := idx.CreateField("edge", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: -100, - Max: 100, - }); err != nil { + if _, err := idx.CreateField("edge", pilosa.OptFieldTypeInt(-100, 100)); err != nil { t.Fatal(err) } @@ -1268,7 +1225,7 @@ func TestExecutor_SetColumnAttrs_ExcludeField(t *testing.T) { hldr := test.Holder{Holder: c[0].Server.Holder()} index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - _, err := index.CreateField("f", pilosa.FieldOptions{}) + _, err := index.CreateField("f", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatalf("creating field: %v", err) } @@ -1352,10 +1309,7 @@ func TestExecutor_Time_Clear_Quantums(t *testing.T) { indexName := strings.ToLower(string(tt.quantum)) index := hldr.MustCreateIndexIfNotExists(indexName, pilosa.IndexOptions{}) // Create field. - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeTime, - TimeQuantum: tt.quantum, - }); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeTime(tt.quantum)); err != nil { t.Fatal(err) } // Populate diff --git a/field.go b/field.go index 76be9cf3f..2b78176e1 100644 --- a/field.go +++ b/field.go @@ -78,6 +78,25 @@ type Field struct { // FieldOption is a functional option type for pilosa.FieldOptions. type FieldOption func(fo *FieldOptions) error +func OptFieldKeys() FieldOption { + return func(fo *FieldOptions) error { + fo.Keys = true + return nil + } +} + +func OptFieldTypeDefault() FieldOption { + return func(fo *FieldOptions) error { + if fo.Type != "" { + return errors.Errorf("field type is already set to: %s", fo.Type) + } + fo.Type = FieldTypeSet + fo.CacheType = DefaultCacheType + fo.CacheSize = DefaultCacheSize + return nil + } +} + func OptFieldTypeSet(cacheType string, cacheSize uint32) FieldOption { return func(fo *FieldOptions) error { if fo.Type != "" { @@ -120,12 +139,19 @@ func OptFieldTypeTime(timeQuantum TimeQuantum) FieldOption { } // NewField returns a new instance of field. -func NewField(path, index, name string, options FieldOptions) (*Field, error) { +func NewField(path, index, name string, opts FieldOption) (*Field, error) { err := validateName(name) if err != nil { return nil, err } + // Apply functional option. + fo := FieldOptions{} + err = opts(&fo) + if err != nil { + return nil, errors.Wrap(err, "applying option") + } + f := &Field{ path: path, index: index, @@ -138,7 +164,7 @@ func NewField(path, index, name string, options FieldOptions) (*Field, error) { broadcaster: NopBroadcaster, Stats: NopStatsClient, - options: applyDefaultOptions(options), + options: applyDefaultOptions(fo), Logger: NopLogger, } diff --git a/field_internal_test.go b/field_internal_test.go index a11b5c394..4ded2bebe 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -153,7 +153,7 @@ func TestBSIGroup_BaseValue(t *testing.T) { // Ensure field can open and retrieve a view. func TestField_DeleteView(t *testing.T) { - f := MustOpenField(FieldOptions{}) + f := MustOpenField(OptFieldTypeDefault()) defer f.Close() viewName := viewStandard + "_v" @@ -190,12 +190,12 @@ type TestField struct { } // NewTestField returns a new instance of TestField d/0. -func NewTestField(options FieldOptions) *TestField { +func NewTestField(opts FieldOption) *TestField { path, err := ioutil.TempDir("", "pilosa-field-") if err != nil { panic(err) } - field, err := NewField(path, "i", "f", options) + field, err := NewField(path, "i", "f", opts) if err != nil { panic(err) } @@ -203,8 +203,8 @@ func NewTestField(options FieldOptions) *TestField { } // MustOpenField returns a new, opened field at a temporary path. Panic on error. -func MustOpenField(options FieldOptions) *TestField { - f := NewTestField(options) +func MustOpenField(opts FieldOption) *TestField { + f := NewTestField(opts) if err := f.Open(); err != nil { panic(err) } @@ -225,7 +225,7 @@ func (f *TestField) Reopen() error { } path, index, name := f.Path(), f.Index(), f.Name() - f.Field, err = NewField(path, index, name, FieldOptions{}) + f.Field, err = NewField(path, index, name, OptFieldTypeDefault()) if err != nil { return err } @@ -253,7 +253,7 @@ func (f *TestField) MustSetBit(row, col uint64, ts ...time.Time) { // Ensure field can open and retrieve a view. func TestField_CreateViewIfNotExists(t *testing.T) { - f := MustOpenField(FieldOptions{}) + f := MustOpenField(OptFieldTypeDefault()) defer f.Close() // Create view. @@ -278,7 +278,7 @@ func TestField_CreateViewIfNotExists(t *testing.T) { } func TestField_SetTimeQuantum(t *testing.T) { - f := MustOpenField(FieldOptions{Type: FieldTypeTime}) + f := MustOpenField(OptFieldTypeTime(TimeQuantum(""))) defer f.Close() // Set & retrieve time quantum. @@ -297,7 +297,7 @@ func TestField_SetTimeQuantum(t *testing.T) { } func TestField_RowTime(t *testing.T) { - f := MustOpenField(FieldOptions{Type: FieldTypeTime}) + f := MustOpenField(OptFieldTypeTime(TimeQuantum(""))) defer f.Close() if err := f.SetTimeQuantum(TimeQuantum("YMDH")); err != nil { diff --git a/field_test.go b/field_test.go index 4b1def96c..520179f52 100644 --- a/field_test.go +++ b/field_test.go @@ -28,11 +28,7 @@ func TestField_SetValue(t *testing.T) { idx := test.MustOpenIndex() defer idx.Close() - f, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 30, - }) + f, err := idx.CreateField("f", pilosa.OptFieldTypeInt(0, 30)) if err != nil { t.Fatal(err) } @@ -65,11 +61,7 @@ func TestField_SetValue(t *testing.T) { idx := test.MustOpenIndex() defer idx.Close() - f, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 0, - Max: 30, - }) + f, err := idx.CreateField("f", pilosa.OptFieldTypeInt(0, 30)) if err != nil { t.Fatal(err) } @@ -102,9 +94,7 @@ func TestField_SetValue(t *testing.T) { idx := test.MustOpenIndex() defer idx.Close() - f, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeSet, - }) + f, err := idx.CreateField("f", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatal(err) } @@ -119,11 +109,7 @@ func TestField_SetValue(t *testing.T) { idx := test.MustOpenIndex() defer idx.Close() - f, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 20, - Max: 30, - }) + f, err := idx.CreateField("f", pilosa.OptFieldTypeInt(20, 30)) if err != nil { t.Fatal(err) } @@ -138,11 +124,7 @@ func TestField_SetValue(t *testing.T) { idx := test.MustOpenIndex() defer idx.Close() - f, err := idx.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 20, - Max: 30, - }) + f, err := idx.CreateField("f", pilosa.OptFieldTypeInt(20, 30)) if err != nil { t.Fatal(err) } @@ -159,7 +141,7 @@ func TestField_NameRestriction(t *testing.T) { if err != nil { panic(err) } - field, err := pilosa.NewField(path, "i", ".meta", pilosa.FieldOptions{}) + field, err := pilosa.NewField(path, "i", ".meta", pilosa.OptFieldTypeDefault()) if field != nil { t.Fatalf("unexpected field name %s", err) } @@ -191,13 +173,13 @@ func TestField_NameValidation(t *testing.T) { panic(err) } for _, name := range validFieldNames { - _, err := pilosa.NewField(path, "i", name, pilosa.FieldOptions{}) + _, err := pilosa.NewField(path, "i", name, pilosa.OptFieldTypeDefault()) if err != nil { t.Fatalf("unexpected field name: %s %s", name, err) } } for _, name := range invalidFieldNames { - _, err := pilosa.NewField(path, "i", name, pilosa.FieldOptions{}) + _, err := pilosa.NewField(path, "i", name, pilosa.OptFieldTypeDefault()) if err == nil { t.Fatalf("expected error on field name: %s", name) } diff --git a/fragment_internal_test.go b/fragment_internal_test.go index eda4cbbdc..4ceb33819 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -750,7 +750,7 @@ func TestFragment_TopN_CacheSize(t *testing.T) { defer index.Close() // Create field. - field, err := index.CreateFieldIfNotExists("f", FieldOptions{CacheType: CacheTypeRanked, CacheSize: cacheSize}) + field, err := index.CreateFieldIfNotExists("f", OptFieldTypeSet(CacheTypeRanked, cacheSize)) if err != nil { t.Fatal(err) } @@ -916,7 +916,7 @@ func TestFragment_RankCache_Persistence(t *testing.T) { defer index.Close() // Create field. - field, err := index.CreateFieldIfNotExists("f", FieldOptions{CacheType: CacheTypeRanked}) + field, err := index.CreateFieldIfNotExists("f", OptFieldTypeSet(CacheTypeRanked, DefaultCacheSize)) if err != nil { t.Fatal(err) } diff --git a/holder.go b/holder.go index e599d21f2..3aae91475 100644 --- a/holder.go +++ b/holder.go @@ -241,7 +241,7 @@ func (h *Holder) applySchema(schema *internal.Schema) error { // Create fields that don't exist. for _, f := range index.Fields { opt := decodeFieldOptions(f.Meta) - field, err := idx.CreateFieldIfNotExists(f.Name, *opt) + field, err := idx.createFieldIfNotExists(f.Name, *opt) if err != nil { return errors.Wrap(err, "creating field") } diff --git a/holder_internal_test.go b/holder_internal_test.go index 425873005..75a3508fd 100644 --- a/holder_internal_test.go +++ b/holder_internal_test.go @@ -60,7 +60,7 @@ func newHolder() *tHolder { // MustCreateFieldIfNotExists returns a given field. Panic on error. func (h *tHolder) MustCreateFieldIfNotExists(index, field string) *Field { - f, err := h.MustCreateIndexIfNotExists(index, IndexOptions{}).CreateFieldIfNotExists(field, FieldOptions{}) + f, err := h.MustCreateIndexIfNotExists(index, IndexOptions{}).CreateFieldIfNotExists(field, OptFieldTypeDefault()) if err != nil { panic(err) } @@ -105,7 +105,7 @@ func TestHolder_Optn(t *testing.T) { if idx, err := h.CreateIndex("foo", IndexOptions{}); err != nil { t.Fatal(err) - } else if field, err := idx.CreateField("bar", FieldOptions{}); err != nil { + } else if field, err := idx.CreateField("bar", OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := field.createViewIfNotExists(viewStandard); err != nil { t.Fatal(err) @@ -129,7 +129,7 @@ func TestHolder_Optn(t *testing.T) { if idx, err := h.CreateIndex("foo", IndexOptions{}); err != nil { t.Fatal(err) - } else if field, err := idx.CreateField("bar", FieldOptions{}); err != nil { + } else if field, err := idx.CreateField("bar", OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := field.createViewIfNotExists(viewStandard); err != nil { t.Fatal(err) @@ -154,7 +154,7 @@ func TestHolder_Optn(t *testing.T) { if idx, err := h.CreateIndex("foo", IndexOptions{}); err != nil { t.Fatal(err) - } else if field, err := idx.CreateField("bar", FieldOptions{}); err != nil { + } else if field, err := idx.CreateField("bar", OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if view, err := field.createViewIfNotExists(viewStandard); err != nil { t.Fatal(err) diff --git a/holder_test.go b/holder_test.go index 078ab55f3..d4886ff30 100644 --- a/holder_test.go +++ b/holder_test.go @@ -98,7 +98,7 @@ func TestHolder_Open(t *testing.T) { if idx, err := h.CreateIndex("foo", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { + } else if _, err := idx.CreateField("bar", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -117,7 +117,7 @@ func TestHolder_Open(t *testing.T) { if idx, err := h.CreateIndex("foo", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { + } else if _, err := idx.CreateField("bar", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -135,7 +135,7 @@ func TestHolder_Open(t *testing.T) { if idx, err := h.CreateIndex("foo", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if _, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { + } else if _, err := idx.CreateField("bar", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -157,7 +157,7 @@ func TestHolder_Open(t *testing.T) { if idx, err := h.CreateIndex("foo", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { + } else if field, err := idx.CreateField("bar", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := field.SetBit(0, 0, nil); err != nil { t.Fatal(err) @@ -178,7 +178,7 @@ func TestHolder_Open(t *testing.T) { if idx, err := h.CreateIndex("foo", pilosa.IndexOptions{}); err != nil { t.Fatal(err) - } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { + } else if field, err := idx.CreateField("bar", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := field.SetBit(0, 0, nil); err != nil { t.Fatal(err) diff --git a/http/client_test.go b/http/client_test.go index fb1105a27..00a9735a6 100644 --- a/http/client_test.go +++ b/http/client_test.go @@ -218,15 +218,10 @@ func TestClient_ImportValue(t *testing.T) { hldr := test.Holder{Holder: holder} fldName := "f" - fo := pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: -100, - Max: 100, - } // Load bitmap into cache to ensure cache gets updated. index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - field, err := index.CreateFieldIfNotExists(fldName, fo) + field, err := index.CreateFieldIfNotExists(fldName, pilosa.OptFieldTypeInt(-100, 100)) if err != nil { t.Fatal(err) } diff --git a/index.go b/index.go index 98f50eced..882bc75d3 100644 --- a/index.go +++ b/index.go @@ -287,7 +287,7 @@ func (i *Index) RecalculateCaches() { } // CreateField creates a field. -func (i *Index) CreateField(name string, opt FieldOptions) (*Field, error) { +func (i *Index) CreateField(name string, opts ...FieldOption) (*Field, error) { i.mu.Lock() defer i.mu.Unlock() @@ -295,11 +295,40 @@ func (i *Index) CreateField(name string, opt FieldOptions) (*Field, error) { if i.fields[name] != nil { return nil, NewConflictError(ErrFieldExists) } - return i.createField(name, opt) + + // Apply functional options. + fo := FieldOptions{} + for _, opt := range opts { + err := opt(&fo) + if err != nil { + return nil, errors.Wrap(err, "applying option") + } + } + + return i.createField(name, fo) } // CreateFieldIfNotExists creates a field with the given options if it doesn't exist. -func (i *Index) CreateFieldIfNotExists(name string, opt FieldOptions) (*Field, error) { +func (i *Index) CreateFieldIfNotExists(name string, opts FieldOption) (*Field, error) { + i.mu.Lock() + defer i.mu.Unlock() + + // Find field in cache first. + if f := i.fields[name]; f != nil { + return f, nil + } + + // Apply functional option. + fo := FieldOptions{} + err := opts(&fo) + 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) { i.mu.Lock() defer i.mu.Unlock() @@ -347,7 +376,7 @@ func (i *Index) createField(name string, opt FieldOptions) (*Field, error) { } func (i *Index) newField(path, name string) (*Field, error) { - f, err := NewField(path, i.name, name, FieldOptions{}) // TODO: NewField should be un-exported along with FieldOptions + f, err := NewField(path, i.name, name, OptFieldTypeDefault()) // TODO: NewField should be un-exported along with FieldOptions if err != nil { return nil, err } diff --git a/index_test.go b/index_test.go index 5490e2978..bc412646a 100644 --- a/index_test.go +++ b/index_test.go @@ -33,7 +33,7 @@ func TestIndex_CreateFieldIfNotExists(t *testing.T) { defer index.Close() // Create field. - f, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{}) + f, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatal(err) } else if f == nil { @@ -41,7 +41,7 @@ func TestIndex_CreateFieldIfNotExists(t *testing.T) { } // Retrieve existing field. - other, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{}) + other, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatal(err) } else if f.Field != other.Field { @@ -61,10 +61,7 @@ func TestIndex_CreateField(t *testing.T) { defer index.Close() // Create field with explicit quantum. - f, err := index.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeTime, - TimeQuantum: pilosa.TimeQuantum("YMDH"), - }) + f, err := index.CreateField("f", pilosa.OptFieldTypeTime(pilosa.TimeQuantum("YMDH"))) if err != nil { t.Fatal(err) } else if q := f.TimeQuantum(); q != pilosa.TimeQuantum("YMDH") { @@ -80,11 +77,7 @@ func TestIndex_CreateField(t *testing.T) { defer index.Close() // Create field with schema and verify it exists. - if f, err := index.CreateField("f", pilosa.FieldOptions{ - Type: pilosa.FieldTypeInt, - Min: 10, - Max: 20, - }); err != nil { + if f, err := index.CreateField("f", pilosa.OptFieldTypeInt(10, 20)); err != nil { t.Fatal(err) } else if !reflect.DeepEqual(f.Type(), pilosa.FieldTypeInt) { t.Fatalf("unexpected type: %#v", f.Type()) @@ -184,7 +177,7 @@ func TestIndex_DeleteField(t *testing.T) { defer index.Close() // Create field. - if _, err := index.CreateFieldIfNotExists("f", pilosa.FieldOptions{}); err != nil { + if _, err := index.CreateFieldIfNotExists("f", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } diff --git a/server.go b/server.go index 56195b427..ba058d134 100644 --- a/server.go +++ b/server.go @@ -455,7 +455,7 @@ func (s *Server) receiveMessage(pb proto.Message) error { return fmt.Errorf("Local Index not found: %s", obj.Index) } opt := decodeFieldOptions(obj.Meta) - _, err := idx.CreateField(obj.Field, *opt) + _, err := idx.createField(obj.Field, *opt) if err != nil { return err } diff --git a/server/handler_test.go b/server/handler_test.go index e49b1eb0f..a5ba8c398 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -62,17 +62,17 @@ func TestHandler_Endpoints(t *testing.T) { i0 := hldr.MustCreateIndexIfNotExists("i0", pilosa.IndexOptions{}) i1 := hldr.MustCreateIndexIfNotExists("i1", pilosa.IndexOptions{}) - if f, err := i0.CreateFieldIfNotExists("f1", pilosa.FieldOptions{}); err != nil { + if f, err := i0.CreateFieldIfNotExists("f1", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } - if f, err := i1.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { + if f, err := i1.CreateFieldIfNotExists("f0", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } - if _, err := i0.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { + if _, err := i0.CreateFieldIfNotExists("f0", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } @@ -401,7 +401,7 @@ func TestHandler_Endpoints(t *testing.T) { t.Run("Field delete", func(t *testing.T) { i := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) - if _, err := i.CreateFieldIfNotExists("f1", pilosa.FieldOptions{}); err != nil { + if _, err := i.CreateFieldIfNotExists("f1", pilosa.OptFieldTypeDefault()); err != nil { t.Fatal(err) } w := httptest.NewRecorder() @@ -453,7 +453,7 @@ func TestHandler_Endpoints(t *testing.T) { } }) - meta, err := i.CreateFieldIfNotExists("meta", pilosa.FieldOptions{}) + meta, err := i.CreateFieldIfNotExists("meta", pilosa.OptFieldTypeDefault()) if err != nil { t.Fatal(err) } diff --git a/test/field.go b/test/field.go index 345deadf8..7a0439026 100644 --- a/test/field.go +++ b/test/field.go @@ -28,12 +28,12 @@ type Field struct { } // NewField returns a new instance of Field d/0. -func NewField(options pilosa.FieldOptions) *Field { +func NewField(opts pilosa.FieldOption) *Field { path, err := ioutil.TempDir("", "pilosa-field-") if err != nil { panic(err) } - field, err := pilosa.NewField(path, "i", "f", options) + field, err := pilosa.NewField(path, "i", "f", opts) if err != nil { panic(err) } @@ -41,8 +41,8 @@ func NewField(options pilosa.FieldOptions) *Field { } // MustOpenField returns a new, opened field at a temporary path. Panic on error. -func MustOpenField(options pilosa.FieldOptions) *Field { - f := NewField(options) +func MustOpenField(opts pilosa.FieldOption) *Field { + f := NewField(opts) if err := f.Open(); err != nil { panic(err) } @@ -63,7 +63,7 @@ func (f *Field) Reopen() error { } path, index, name := f.Path(), f.Index(), f.Name() - f.Field, err = pilosa.NewField(path, index, name, pilosa.FieldOptions{}) + f.Field, err = pilosa.NewField(path, index, name, pilosa.OptFieldTypeDefault()) if err != nil { return err } @@ -76,7 +76,7 @@ func (f *Field) Reopen() error { // Ensure field can set its cache func TestField_SetCacheSize(t *testing.T) { - f := MustOpenField(pilosa.FieldOptions{}) + f := MustOpenField(pilosa.OptFieldTypeDefault()) defer f.Close() cacheSize := uint32(100) diff --git a/test/holder.go b/test/holder.go index 94d1c778f..9cc96fe14 100644 --- a/test/holder.go +++ b/test/holder.go @@ -83,7 +83,7 @@ func (h *Holder) MustCreateIndexIfNotExists(index string, opt pilosa.IndexOption // MustCreateFieldIfNotExists returns a given field. Panic on error. func (h *Holder) MustCreateFieldIfNotExists(index, field string) *Field { - f, err := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}).CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}).CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } @@ -93,7 +93,7 @@ func (h *Holder) MustCreateFieldIfNotExists(index, field string) *Field { // Row returns a Row for a given field. func (h *Holder) Row(index, field string, rowID uint64) *pilosa.Row { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := idx.CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } @@ -106,7 +106,7 @@ func (h *Holder) Row(index, field string, rowID uint64) *pilosa.Row { func (h *Holder) RowAttrStore(index, field string) pilosa.AttrStore { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := idx.CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } @@ -115,7 +115,7 @@ func (h *Holder) RowAttrStore(index, field string) pilosa.AttrStore { func (h *Holder) RowTime(index, field string, rowID uint64, t time.Time, quantum string) *pilosa.Row { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := idx.CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } @@ -129,7 +129,7 @@ func (h *Holder) RowTime(index, field string, rowID uint64, t time.Time, quantum // SetBit clears a bit on the given field. func (h *Holder) SetBit(index, field string, rowID, columnID uint64) { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := idx.CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } @@ -142,7 +142,7 @@ func (h *Holder) SetBit(index, field string, rowID, columnID uint64) { // ClearBit clears a bit on the given field. func (h *Holder) ClearBit(index, field string, rowID, columnID uint64) { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) + f, err := idx.CreateFieldIfNotExists(field, pilosa.OptFieldTypeDefault()) if err != nil { panic(err) } diff --git a/test/index.go b/test/index.go index 69c18682c..9b7c1684b 100644 --- a/test/index.go +++ b/test/index.go @@ -74,8 +74,8 @@ func (i *Index) Reopen() error { } // CreateField creates a field with the given options. -func (i *Index) CreateField(name string, opt pilosa.FieldOptions) (*Field, error) { - f, err := i.Index.CreateField(name, opt) +func (i *Index) CreateField(name string, opts ...pilosa.FieldOption) (*Field, error) { + f, err := i.Index.CreateField(name, opts...) if err != nil { return nil, err } @@ -83,8 +83,8 @@ func (i *Index) CreateField(name string, opt pilosa.FieldOptions) (*Field, error } // CreateFieldIfNotExists creates a field with the given options if it doesn't exist. -func (i *Index) CreateFieldIfNotExists(name string, opt pilosa.FieldOptions) (*Field, error) { - f, err := i.Index.CreateFieldIfNotExists(name, opt) +func (i *Index) CreateFieldIfNotExists(name string, opts pilosa.FieldOption) (*Field, error) { + f, err := i.Index.CreateFieldIfNotExists(name, opts) if err != nil { return nil, err } diff --git a/utils_internal_test.go b/utils_internal_test.go index ca7bd1fa7..bbcf300cd 100644 --- a/utils_internal_test.go +++ b/utils_internal_test.go @@ -104,13 +104,13 @@ func (t *ClusterCluster) CreateIndex(name string) error { return nil } -func (t *ClusterCluster) CreateField(index, field string, opt FieldOptions) error { +func (t *ClusterCluster) CreateField(index, field string, opts FieldOption) error { for _, c := range t.Clusters { idx, err := c.holder.CreateIndexIfNotExists(index, IndexOptions{}) if err != nil { return err } - if _, err := idx.CreateField(field, opt); err != nil { + if _, err := idx.CreateField(field, opts); err != nil { return err } } From 3e288aa3910e7f3b50bd67a6536481bf7de4b31a Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 4 Jul 2018 21:39:01 -0500 Subject: [PATCH 4/5] un-export field.FieldOptions --- api.go | 2 +- field.go | 46 +++++++++++++++++++++++----------------------- http/handler.go | 2 +- index.go | 10 +++++----- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/api.go b/api.go index f0e2dbfb0..f9efbdb00 100644 --- a/api.go +++ b/api.go @@ -244,7 +244,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str } // Apply functional option. - fo := FieldOptions{} + fo := fieldOptions{} err := opts(&fo) if err != nil { return nil, errors.Wrap(err, "applying option") diff --git a/field.go b/field.go index 2b78176e1..f2cb711c2 100644 --- a/field.go +++ b/field.go @@ -68,25 +68,25 @@ type Field struct { Stats StatsClient // Field options. - options FieldOptions + options fieldOptions bsiGroups []*bsiGroup Logger Logger } -// FieldOption is a functional option type for pilosa.FieldOptions. -type FieldOption func(fo *FieldOptions) error +// FieldOption is a functional option type for pilosa.fieldOptions. +type FieldOption func(fo *fieldOptions) error func OptFieldKeys() FieldOption { - return func(fo *FieldOptions) error { + return func(fo *fieldOptions) error { fo.Keys = true return nil } } func OptFieldTypeDefault() FieldOption { - return func(fo *FieldOptions) error { + return func(fo *fieldOptions) error { if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) } @@ -98,7 +98,7 @@ func OptFieldTypeDefault() FieldOption { } func OptFieldTypeSet(cacheType string, cacheSize uint32) FieldOption { - return func(fo *FieldOptions) error { + return func(fo *fieldOptions) error { if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) } @@ -110,7 +110,7 @@ func OptFieldTypeSet(cacheType string, cacheSize uint32) FieldOption { } func OptFieldTypeInt(min, max int64) FieldOption { - return func(fo *FieldOptions) error { + return func(fo *fieldOptions) error { if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) } @@ -125,7 +125,7 @@ func OptFieldTypeInt(min, max int64) FieldOption { } func OptFieldTypeTime(timeQuantum TimeQuantum) FieldOption { - return func(fo *FieldOptions) error { + return func(fo *fieldOptions) error { if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) } @@ -146,7 +146,7 @@ func NewField(path, index, name string, opts FieldOption) (*Field, error) { } // Apply functional option. - fo := FieldOptions{} + fo := fieldOptions{} err = opts(&fo) if err != nil { return nil, errors.Wrap(err, "applying option") @@ -240,7 +240,7 @@ func (f *Field) CacheSize() uint32 { } // Options returns all options for this field. -func (f *Field) Options() FieldOptions { +func (f *Field) Options() fieldOptions { f.mu.RLock() defer f.mu.RUnlock() return f.options @@ -358,7 +358,7 @@ func (f *Field) saveMeta() error { } // applyOptions configures the field based on opt. -func (f *Field) applyOptions(opt FieldOptions) error { +func (f *Field) applyOptions(opt fieldOptions) error { switch opt.Type { case FieldTypeSet, "": f.options.Type = FieldTypeSet @@ -1102,7 +1102,7 @@ func (f *Field) ImportValue(columnIDs []uint64, values []int64) error { func (f *Field) MarshalJSON() ([]byte, error) { thing := struct { Name string - Options FieldOptions + Options fieldOptions Views []*viewInfo }{ Name: f.Name(), @@ -1142,7 +1142,7 @@ func (p fieldSlice) Less(i, j int) bool { return p[i].Name() < p[j].Name() } // FieldInfo represents schema information for a field. type FieldInfo struct { Name string `json:"name"` - Options FieldOptions `json:"options"` + Options fieldOptions `json:"options"` Views []*viewInfo `json:"views,omitempty"` } @@ -1152,8 +1152,8 @@ func (p fieldInfoSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } func (p fieldInfoSlice) Len() int { return len(p) } func (p fieldInfoSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } -// FieldOptions represents options to set when initializing a field. -type FieldOptions struct { +// fieldOptions represents options to set when initializing a field. +type fieldOptions struct { Type string `json:"type,omitempty"` CacheType string `json:"cacheType,omitempty"` CacheSize uint32 `json:"cacheSize,omitempty"` @@ -1163,11 +1163,11 @@ type FieldOptions struct { Keys bool `json:"keys,omitempty"` } -// applyDefaultOptions returns a new FieldOptions object +// applyDefaultOptions returns a new fieldOptions object // with default values if o does not contain a valid type. -func applyDefaultOptions(o FieldOptions) FieldOptions { +func applyDefaultOptions(o fieldOptions) fieldOptions { if o.Type == "" { - return FieldOptions{ + return fieldOptions{ Type: DefaultFieldType, CacheType: DefaultCacheType, CacheSize: DefaultCacheSize, @@ -1177,11 +1177,11 @@ func applyDefaultOptions(o FieldOptions) FieldOptions { } // Encode converts o into its internal representation. -func (o *FieldOptions) Encode() *internal.FieldOptions { +func (o *fieldOptions) Encode() *internal.FieldOptions { return encodeFieldOptions(o) } -func encodeFieldOptions(o *FieldOptions) *internal.FieldOptions { +func encodeFieldOptions(o *fieldOptions) *internal.FieldOptions { if o == nil { return nil } @@ -1196,11 +1196,11 @@ func encodeFieldOptions(o *FieldOptions) *internal.FieldOptions { } } -func decodeFieldOptions(options *internal.FieldOptions) *FieldOptions { +func decodeFieldOptions(options *internal.FieldOptions) *fieldOptions { if options == nil { return nil } - return &FieldOptions{ + return &fieldOptions{ Type: options.Type, CacheType: options.CacheType, CacheSize: options.CacheSize, @@ -1211,7 +1211,7 @@ func decodeFieldOptions(options *internal.FieldOptions) *FieldOptions { } } -func (o *FieldOptions) MarshalJSON() ([]byte, error) { +func (o *fieldOptions) MarshalJSON() ([]byte, error) { switch o.Type { case FieldTypeSet: return json.Marshal(struct { diff --git a/http/handler.go b/http/handler.go index 9195f05e9..1089c5329 100644 --- a/http/handler.go +++ b/http/handler.go @@ -671,7 +671,7 @@ type postFieldRequest struct { Options fieldOptions `json:"options"` } -// fieldOptions tracks pilosa.FieldOptions. It is made up of pointers to values, +// fieldOptions tracks pilosa.fieldOptions. It is made up of pointers to values, // and used for input validation. type fieldOptions struct { Type string `json:"type,omitempty"` diff --git a/index.go b/index.go index 882bc75d3..bf0e55290 100644 --- a/index.go +++ b/index.go @@ -297,7 +297,7 @@ func (i *Index) CreateField(name string, opts ...FieldOption) (*Field, error) { } // Apply functional options. - fo := FieldOptions{} + fo := fieldOptions{} for _, opt := range opts { err := opt(&fo) if err != nil { @@ -319,7 +319,7 @@ func (i *Index) CreateFieldIfNotExists(name string, opts FieldOption) (*Field, e } // Apply functional option. - fo := FieldOptions{} + fo := fieldOptions{} err := opts(&fo) if err != nil { return nil, errors.Wrap(err, "applying option") @@ -328,7 +328,7 @@ func (i *Index) CreateFieldIfNotExists(name string, opts FieldOption) (*Field, e 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() @@ -340,7 +340,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) { @@ -376,7 +376,7 @@ func (i *Index) createField(name string, opt FieldOptions) (*Field, error) { } func (i *Index) newField(path, name string) (*Field, error) { - f, err := NewField(path, i.name, name, OptFieldTypeDefault()) // TODO: NewField should be un-exported along with FieldOptions + f, err := NewField(path, i.name, name, OptFieldTypeDefault()) if err != nil { return nil, err } From bae5ee36efecc591669a1a2f1331c86306b976ca Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 4 Jul 2018 22:24:05 -0500 Subject: [PATCH 5/5] add support for OptFieldKeys() to http field creation --- api.go | 14 ++++++++------ http/handler.go | 15 ++++++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/api.go b/api.go index f9efbdb00..8ca101c61 100644 --- a/api.go +++ b/api.go @@ -238,16 +238,18 @@ func (api *API) DeleteIndex(ctx context.Context, indexName string) error { // CreateField makes the named field in the named index with the given options. // This method currently only takes a single functional option, but that may be // changed in the future to support multiple options. -func (api *API) CreateField(ctx context.Context, indexName string, fieldName string, opts FieldOption) (*Field, error) { +func (api *API) CreateField(ctx context.Context, indexName string, fieldName string, opts ...FieldOption) (*Field, error) { if err := api.validate(apiCreateField); err != nil { return nil, errors.Wrap(err, "validating api method") } - // Apply functional option. + // Apply functional options. fo := fieldOptions{} - err := opts(&fo) - if err != nil { - return nil, errors.Wrap(err, "applying option") + for _, opt := range opts { + err := opt(&fo) + if err != nil { + return nil, errors.Wrap(err, "applying option") + } } // Find index. @@ -257,7 +259,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str } // Create field. - field, err := index.CreateField(fieldName, opts) + field, err := index.CreateField(fieldName, opts...) if err != nil { return nil, errors.Wrap(err, "creating field") } diff --git a/http/handler.go b/http/handler.go index 1089c5329..8b2015094 100644 --- a/http/handler.go +++ b/http/handler.go @@ -653,17 +653,22 @@ func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { } // Convert json options into functional options. - var fos pilosa.FieldOption + var fos []pilosa.FieldOption switch req.Options.Type { case pilosa.FieldTypeSet: - fos = pilosa.OptFieldTypeSet(*req.Options.CacheType, *req.Options.CacheSize) + fos = append(fos, pilosa.OptFieldTypeSet(*req.Options.CacheType, *req.Options.CacheSize)) case pilosa.FieldTypeInt: - fos = pilosa.OptFieldTypeInt(*req.Options.Min, *req.Options.Max) + fos = append(fos, pilosa.OptFieldTypeInt(*req.Options.Min, *req.Options.Max)) case pilosa.FieldTypeTime: - fos = pilosa.OptFieldTypeTime(*req.Options.TimeQuantum) + fos = append(fos, pilosa.OptFieldTypeTime(*req.Options.TimeQuantum)) + } + if req.Options.Keys != nil { + if *req.Options.Keys { + fos = append(fos, pilosa.OptFieldKeys()) + } } - _, err = h.API.CreateField(r.Context(), indexName, fieldName, fos) + _, err = h.API.CreateField(r.Context(), indexName, fieldName, fos...) resp.write(w, err) }