From f21f72189b496b5d812dc26d13d49eea72bb12b0 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Wed, 20 May 2015 14:15:33 -0600 Subject: [PATCH 1/4] fmt --- influxql/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/influxql/engine.go b/influxql/engine.go index 652e40b0492..ad760d0ea84 100644 --- a/influxql/engine.go +++ b/influxql/engine.go @@ -702,7 +702,7 @@ func (m *MapReduceJob) processRawResults(values []*rawQueryMapOutput) *Row { selectNames[0], selectNames[i] = selectNames[i], selectNames[0] } hasTime = true - break + break } } From c291b3a47a644bf06b635a18c81bab439d598172 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Wed, 20 May 2015 14:15:44 -0600 Subject: [PATCH 2/4] add custom error message for select distict against tags --- CHANGELOG.md | 1 + cmd/influxd/server_integration_test.go | 42 ++++++++++++++++++++++---- tx.go | 7 +++++ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8f64e6a5c3..077f857a6e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - [#2596](https://github.com/influxdb/influxdb/pull/2596): RC30: `panic: runtime error: index out of range` when insert data points. - [#2592](https://github.com/influxdb/influxdb/pull/2592): Should return an error if user attempts to group by a field. - [#2499](https://github.com/influxdb/influxdb/pull/2499): Issuing a select query with tag as a values causes panic. +- [#2612](https://github.com/influxdb/influxdb/pull/2612): Query planner should validate distinct is passed a field. ## PRs - [#2569](https://github.com/influxdb/influxdb/pull/2569): Add derivative functions diff --git a/cmd/influxd/server_integration_test.go b/cmd/influxd/server_integration_test.go index 4784efce3bf..1be64732aa2 100644 --- a/cmd/influxd/server_integration_test.go +++ b/cmd/influxd/server_integration_test.go @@ -705,29 +705,59 @@ func runTestsData(t *testing.T, testName string, nodes Cluster, database, retent }, { reset: true, - name: "distincts", + name: "distinct as call", write: `{"database" : "%DB%", "retentionPolicy" : "%RP%", "points": [ - {"name": "cpu", "time": "2000-01-01T00:00:00Z", "fields": {"value": 30}}, - {"name": "cpu", "time": "2000-01-01T00:00:10Z", "fields": {"value": 20}}, - {"name": "cpu", "time": "2000-01-01T00:00:20Z", "fields": {"value": 30}}, - {"name": "cpu", "time": "2000-01-01T00:00:30Z", "fields": {"value": 100}} + {"name": "cpu", "time": "2000-01-01T00:00:00Z", "tags": {"host": "server01"}, "fields": {"value": 30}}, + {"name": "cpu", "time": "2000-01-01T00:00:10Z", "tags": {"host": "server02"}, "fields": {"value": 20}}, + {"name": "cpu", "time": "2000-01-01T00:00:20Z", "tags": {"host": "server03"}, "fields": {"value": 30}}, + {"name": "cpu", "time": "2000-01-01T00:00:30Z", "tags": {"host": "server03"}, "fields": {"value": 100}} ]}`, query: `SELECT distinct(value) FROM cpu`, queryDb: "%DB%", expected: `{"results":[{"series":[{"name":"cpu","columns":["time","distinct"],"values":[["1970-01-01T00:00:00Z",[20,30,100]]]}]}]}`, }, { - name: "distincts alt syntax", + name: "distinct alt syntax", query: `SELECT distinct value FROM cpu`, queryDb: "%DB%", expected: `{"results":[{"series":[{"name":"cpu","columns":["time","distinct"],"values":[["1970-01-01T00:00:00Z",[20,30,100]]]}]}]}`, }, + { + name: "distinct select tag", + query: `SELECT distinct(host) FROM cpu`, + queryDb: "%DB%", + expected: `{"results":[{"error":"host isn't a field on measurement cpu; to query the unique values for a tag use SHOW TAG VALUES FROM cpu WITH KEY = \"host\""}]}`, + }, + { + name: "distinct alt select tag", + query: `SELECT distinct host FROM cpu`, + queryDb: "%DB%", + expected: `{"results":[{"error":"host isn't a field on measurement cpu; to query the unique values for a tag use SHOW TAG VALUES FROM cpu WITH KEY = \"host\""}]}`, + }, { name: "count distinct", query: `SELECT count(distinct value) FROM cpu`, queryDb: "%DB%", expected: `{"results":[{"series":[{"name":"cpu","columns":["time","count"],"values":[["1970-01-01T00:00:00Z",3]]}]}]}`, }, + { + name: "count distinct as call", + query: `SELECT count(distinct(value)) FROM cpu`, + queryDb: "%DB%", + expected: `{"results":[{"series":[{"name":"cpu","columns":["time","count"],"values":[["1970-01-01T00:00:00Z",3]]}]}]}`, + }, + { + name: "count distinct select tag", + query: `SELECT count(distinct host) FROM cpu`, + queryDb: "%DB%", + expected: `{"results":[{"error":"host isn't a field on measurement cpu"}]}`, + }, + { + name: "count distinct as call select tag", + query: `SELECT count(distinct(host)) FROM cpu`, + queryDb: "%DB%", + expected: `{"results":[{"error":"host isn't a field on measurement cpu"}]}`, + }, { reset: true, name: "aggregations", diff --git a/tx.go b/tx.go index 16c679f7f4b..4ce365daeac 100644 --- a/tx.go +++ b/tx.go @@ -374,6 +374,8 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) l.chunkSize = chunkSize l.tmin = startingTime + var isDistinct bool + // determine if this is a raw data query with a single field, multiple fields, or an aggregate var fieldName string if c == nil { // its a raw data query @@ -404,12 +406,17 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) default: return fmt.Errorf("aggregate call didn't contain a field %s", c.String()) } + + isDistinct = c.Name == "distinct" } // set up the field info if a specific field was set for this mapper if fieldName != "" { f := l.decoder.FieldByName(fieldName) if f == nil { + if isDistinct { + return fmt.Errorf("%s isn't a field on measurement %s; to query the unique values for a tag use SHOW TAG VALUES FROM %s WITH KEY = %q", fieldName, l.job.MeasurementName, l.job.MeasurementName, fieldName) + } return fmt.Errorf("%s isn't a field on measurement %s", fieldName, l.job.MeasurementName) } l.fieldID = f.ID From 68f4b4cc458c51e87a196ca215a40d1d0a5ffd6e Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Wed, 20 May 2015 14:26:57 -0600 Subject: [PATCH 3/4] refactor out single use variable --- tx.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tx.go b/tx.go index 4ce365daeac..b9d58320163 100644 --- a/tx.go +++ b/tx.go @@ -374,8 +374,6 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) l.chunkSize = chunkSize l.tmin = startingTime - var isDistinct bool - // determine if this is a raw data query with a single field, multiple fields, or an aggregate var fieldName string if c == nil { // its a raw data query @@ -407,14 +405,13 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) return fmt.Errorf("aggregate call didn't contain a field %s", c.String()) } - isDistinct = c.Name == "distinct" } // set up the field info if a specific field was set for this mapper if fieldName != "" { f := l.decoder.FieldByName(fieldName) if f == nil { - if isDistinct { + if c.Name == "distinct" { return fmt.Errorf("%s isn't a field on measurement %s; to query the unique values for a tag use SHOW TAG VALUES FROM %s WITH KEY = %q", fieldName, l.job.MeasurementName, l.job.MeasurementName, fieldName) } return fmt.Errorf("%s isn't a field on measurement %s", fieldName, l.job.MeasurementName) From 5e9f7064f4d46a7d47361947f820af2ce8be7ad2 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Wed, 20 May 2015 15:02:39 -0600 Subject: [PATCH 4/4] custom errors for distinct vs. count distinct --- cmd/influxd/server_integration_test.go | 4 ++-- tx.go | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cmd/influxd/server_integration_test.go b/cmd/influxd/server_integration_test.go index 1be64732aa2..09f082708d1 100644 --- a/cmd/influxd/server_integration_test.go +++ b/cmd/influxd/server_integration_test.go @@ -750,13 +750,13 @@ func runTestsData(t *testing.T, testName string, nodes Cluster, database, retent name: "count distinct select tag", query: `SELECT count(distinct host) FROM cpu`, queryDb: "%DB%", - expected: `{"results":[{"error":"host isn't a field on measurement cpu"}]}`, + expected: `{"results":[{"error":"host isn't a field on measurement cpu; count(distinct) on tags isn't yet supported"}]}`, }, { name: "count distinct as call select tag", query: `SELECT count(distinct(host)) FROM cpu`, queryDb: "%DB%", - expected: `{"results":[{"error":"host isn't a field on measurement cpu"}]}`, + expected: `{"results":[{"error":"host isn't a field on measurement cpu; count(distinct) on tags isn't yet supported"}]}`, }, { reset: true, diff --git a/tx.go b/tx.go index b9d58320163..892b3bd0e85 100644 --- a/tx.go +++ b/tx.go @@ -374,6 +374,8 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) l.chunkSize = chunkSize l.tmin = startingTime + var isCountDistinct bool + // determine if this is a raw data query with a single field, multiple fields, or an aggregate var fieldName string if c == nil { // its a raw data query @@ -400,21 +402,27 @@ func (l *LocalMapper) Begin(c *influxql.Call, startingTime int64, chunkSize int) if c.Name != "count" { return fmt.Errorf("aggregate call didn't contain a field %s", c.String()) } + isCountDistinct = true fieldName = lit.Val default: return fmt.Errorf("aggregate call didn't contain a field %s", c.String()) } + isCountDistinct = isCountDistinct || (c.Name == "count" && nested.Name == "distinct") } // set up the field info if a specific field was set for this mapper if fieldName != "" { f := l.decoder.FieldByName(fieldName) if f == nil { - if c.Name == "distinct" { + switch { + case c.Name == "distinct": return fmt.Errorf("%s isn't a field on measurement %s; to query the unique values for a tag use SHOW TAG VALUES FROM %s WITH KEY = %q", fieldName, l.job.MeasurementName, l.job.MeasurementName, fieldName) + case isCountDistinct: + return fmt.Errorf("%s isn't a field on measurement %s; count(distinct) on tags isn't yet supported", fieldName, l.job.MeasurementName) + default: + return fmt.Errorf("%s isn't a field on measurement %s", fieldName, l.job.MeasurementName) } - return fmt.Errorf("%s isn't a field on measurement %s", fieldName, l.job.MeasurementName) } l.fieldID = f.ID l.fieldName = f.Name