From 92d70005710156a6c2e0d3d9f32fa58821f38892 Mon Sep 17 00:00:00 2001
From: Yuce Tekol <yucetekol@gmail.com>
Date: Thu, 14 Sep 2017 11:03:05 +0300
Subject: [PATCH 1/3] SetColumnAttrs excludes frame attribute; resolves #792

---
 executor.go      |  1 +
 executor_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/executor.go b/executor.go
index 018728f53..03ce03aaa 100644
--- a/executor.go
+++ b/executor.go
@@ -1252,6 +1252,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)
+	}
+
+}

From b9a3197c2c9f1ef542020ea21296b72b4b2972e3 Mon Sep 17 00:00:00 2001
From: Yuce Tekol <yucetekol@gmail.com>
Date: Thu, 14 Sep 2017 11:10:22 +0300
Subject: [PATCH 2/3] Updated docs

---
 docs/query-language.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/query-language.md b/docs/query-language.md
index c9d5f1e05..666ba60bf 100644
--- a/docs/query-language.md
+++ b/docs/query-language.md
@@ -139,18 +139,18 @@ SetColumnAttrs(<frame=STRING>, <ROW_LABEL=UINT>,
 
 **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. Note that, `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.

From a27262b242a590df2d282d3c7fe1981316569d6b Mon Sep 17 00:00:00 2001
From: Yuce Tekol <yucetekol@gmail.com>
Date: Mon, 18 Sep 2017 16:41:13 +0300
Subject: [PATCH 3/3] update

---
 docs/query-language.md | 2 +-
 executor.go            | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/query-language.md b/docs/query-language.md
index 666ba60bf..7c54c3ba8 100644
--- a/docs/query-language.md
+++ b/docs/query-language.md
@@ -139,7 +139,7 @@ SetColumnAttrs(<frame=STRING>, <ROW_LABEL=UINT>,
 
 **Result Type:** null
 
-SetColumnAttrs queries always return `null` upon success. Setting a value of `null`, without quotes, deletes an attribute. Note that, `frame` cannot be used as an attribute name.
+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:**
 
diff --git a/executor.go b/executor.go
index 03ce03aaa..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.