diff --git a/docs/query-language.md b/docs/query-language.md index c9d5f1e05..7c54c3ba8 100644 --- a/docs/query-language.md +++ b/docs/query-language.md @@ -139,18 +139,18 @@ SetColumnAttrs(, , **Result Type:** null -SetColumnAttrs queries always return `null` upon success. Setting a value of `null`, without quotes, deletes an attribute. +SetColumnAttrs queries always return `null` upon success. Setting a value of `null`, without quotes, deletes an attribute. To avoid confusion, `frame` cannot be used as an attribute name. **Examples:** ``` -SetColumnAttrs(frame="stargazer", repo_id=10, stars=123, url="http://projects.pilosa.com/10", active=true) +SetColumnAttrs(repo_id=10, stars=123, url="http://projects.pilosa.com/10", active=true) ``` Set url value and active status for project 10. These are arbitrary key/value pairs which have no meaning to Pilosa. You can see the attributes you've set on a column with a [Bitmap]({{< ref "query-language.md#bitmap" >}}) query like so `Bitmap(frame="stargazer", repo_id=10)`. ``` -SetColumnAttrs(frame="stargazer", repo_id=10, url=null) +SetColumnAttrs(repo_id=10, url=null) ``` Delete url value for repo 10. diff --git a/executor.go b/executor.go index 018728f53..1507c3050 100644 --- a/executor.go +++ b/executor.go @@ -1040,6 +1040,8 @@ func (e *Executor) executeSetFieldValue(ctx context.Context, index string, c *pq // Copy args and remove reserved fields. args := pql.CopyArgs(c.Args) delete(args, "frame") + // While frame could technically work as a ColumnAttr argument, we are treating it as a reserved word primarily to avoid confusion. + // Also, if we ever need to make ColumnAttrs frame-specific, then having this reserved word prevents backward incompatibility. delete(args, columnLabel) // Set values. @@ -1252,6 +1254,7 @@ func (e *Executor) executeSetColumnAttrs(ctx context.Context, index string, c *p // Copy args and remove reserved fields. attrs := pql.CopyArgs(c.Args) delete(attrs, colName) + delete(attrs, "frame") // Set attributes. if err := idx.ColumnAttrStore().SetAttrs(id, attrs); err != nil { diff --git a/executor_test.go b/executor_test.go index b426b3875..482903bc2 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1062,3 +1062,50 @@ func TestExecutor_Execute_ErrMaxWritesPerRequest(t *testing.T) { t.Fatalf("unexpected error: %s", err) } } + +// Ensure SetColumnAttrs doesn't save `frame` as an attribute +func TestExectutor_SetColumnAttrs_ExcludeFrame(t *testing.T) { + hldr := test.MustOpenHolder() + defer hldr.Close() + index := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) + index.CreateFrame("f", pilosa.FrameOptions{InverseEnabled: true}) + targetAttrs := map[string]interface{}{ + "foo": "bar", + } + e := test.NewExecutor(hldr.Holder, test.NewCluster(1)) + + // SetColumnAttrs call should exclude the frame attribute + _, err := e.Execute(context.Background(), "i", test.MustParse("SetBit(frame='f', rowID=1, columnID=10)"), nil, nil) + if err != nil { + t.Fatal(err) + } + _, err = e.Execute(context.Background(), "i", test.MustParse("SetColumnAttrs(frame='f', columnID=10, foo='bar')"), nil, nil) + if err != nil { + t.Fatal(err) + } + attrs, err := index.ColumnAttrStore().Attrs(10) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(attrs, targetAttrs) { + t.Fatalf("%#v != %#v", targetAttrs, attrs) + } + + // SetColumnAttrs call should not break if frame is not specified + _, err = e.Execute(context.Background(), "i", test.MustParse("SetBit(frame='f', rowID=1, columnID=20)"), nil, nil) + if err != nil { + t.Fatal(err) + } + _, err = e.Execute(context.Background(), "i", test.MustParse("SetColumnAttrs(columnID=20, foo='bar')"), nil, nil) + if err != nil { + t.Fatal(err) + } + attrs, err = index.ColumnAttrStore().Attrs(20) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(attrs, targetAttrs) { + t.Fatalf("%#v != %#v", targetAttrs, attrs) + } + +}