From eda74a25f11005f3ea016d8aec957915de2ed13f Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Thu, 12 Mar 2015 18:20:58 -0600 Subject: [PATCH 01/14] allow offset 0 --- influxql/parser.go | 32 ++++++++++++++++---------------- influxql/parser_test.go | 12 ++++++++++++ server_test.go | 8 ++++++++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/influxql/parser.go b/influxql/parser.go index c21215d899..dfc8ace39b 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -564,22 +564,22 @@ func (p *Parser) parseSelectStatement(tr targetRequirement) (*SelectStatement, e } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { return nil, err } // Parse series limit: "SLIMIT ". - if stmt.SLimit, err = p.parseOptionalTokenAndInt(SLIMIT); err != nil { + if stmt.SLimit, err = p.parseOptionalTokenAndInt(SLIMIT, false); err != nil { return nil, err } // Parse series offset: "SOFFSET ". - if stmt.SOffset, err = p.parseOptionalTokenAndInt(SOFFSET); err != nil { + if stmt.SOffset, err = p.parseOptionalTokenAndInt(SOFFSET, false); err != nil { return nil, err } @@ -679,12 +679,12 @@ func (p *Parser) parseShowSeriesStatement() (*ShowSeriesStatement, error) { } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, true); err != nil { return nil, err } @@ -708,12 +708,12 @@ func (p *Parser) parseShowMeasurementsStatement() (*ShowMeasurementsStatement, e } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { return nil, err } @@ -760,12 +760,12 @@ func (p *Parser) parseShowTagKeysStatement() (*ShowTagKeysStatement, error) { } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { return nil, err } @@ -803,12 +803,12 @@ func (p *Parser) parseShowTagValuesStatement() (*ShowTagValuesStatement, error) } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { return nil, err } @@ -883,12 +883,12 @@ func (p *Parser) parseShowFieldKeysStatement() (*ShowFieldKeysStatement, error) } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { return nil, err } @@ -1413,7 +1413,7 @@ func (p *Parser) parseFill() (FillOption, interface{}, error) { // parseOptionalTokenAndInt parses the specified token followed // by an int, if it exists. -func (p *Parser) parseOptionalTokenAndInt(t Token) (int, error) { +func (p *Parser) parseOptionalTokenAndInt(t Token, allowZero bool) (int, error) { // Check if the token exists. if tok, _, _ := p.scanIgnoreWhitespace(); tok != t { p.unscan() @@ -1435,7 +1435,7 @@ func (p *Parser) parseOptionalTokenAndInt(t Token) (int, error) { // Parse number. n, _ := strconv.ParseInt(lit, 10, 64) - if n < 1 { + if n < 1 && !allowZero { msg := fmt.Sprintf("%s must be > 0", t.String()) return 0, &ParseError{Message: msg, Pos: pos} } diff --git a/influxql/parser_test.go b/influxql/parser_test.go index ebbd806e87..5cb6b6520b 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -244,6 +244,18 @@ func TestParser_ParseStatement(t *testing.T) { stmt: &influxql.ShowSeriesStatement{}, }, + // SHOW SERIES with OFFSET 0 + { + s: `SHOW SERIES OFFSET 0`, + stmt: &influxql.ShowSeriesStatement{Offset: 0}, + }, + + // SHOW SERIES with LIMIT 2 OFFSET 0 + { + s: `SHOW SERIES LIMIT 2 OFFSET 0`, + stmt: &influxql.ShowSeriesStatement{Offset: 0, Limit: 2}, + }, + // SHOW SERIES WHERE with ORDER BY and LIMIT { s: `SHOW SERIES WHERE region = 'uswest' ORDER BY ASC, field1, field2 DESC LIMIT 10`, diff --git a/server_test.go b/server_test.go index 510747319d..9de930c479 100644 --- a/server_test.go +++ b/server_test.go @@ -1609,6 +1609,14 @@ func TestServer_ShowSeriesLimitOffset(t *testing.T) { t.Fatalf("unexpected row count: %d", len(res.Series)) } + // Select data from the server. + results = s.ExecuteQuery(MustParseQuery(`SHOW SERIES LIMIT 4 OFFSET 0`), "foo", nil) + if res := results.Results[0]; res.Err != nil { + t.Fatalf("unexpected error: %s", res.Err) + } else if len(res.Series) != 2 { + t.Fatalf("unexpected row count: %d", len(res.Series)) + } + // Select data from the server. results = s.ExecuteQuery(MustParseQuery(`SHOW SERIES LIMIT 20`), "foo", nil) if res := results.Results[0]; res.Err != nil { From bdfb9bfa0b001d1bd34f1427f280ad3a9200c78a Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Thu, 12 Mar 2015 18:23:51 -0600 Subject: [PATCH 02/14] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e9b50956..2468d8ae02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - [#1930](https://github.com/influxdb/influxdb/pull/1930): Auto create database for graphite if not specified. - [#1908](https://github.com/influxdb/influxdb/pull/1908): Cosmetic CLI output fixes. - [#1931](https://github.com/influxdb/influxdb/pull/1931): Add default column to SHOW RETENTION POLICIES. +- [#1937](https://github.com/influxdb/influxdb/pull/1937): OFFSET should be allowed to be 0. ### Features - [#1902](https://github.com/influxdb/influxdb/pull/1902): Enforce retention policies to have a minimum duration. From 08b15d937092bcd6d0a4a871284a63c796ef9162 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Thu, 12 Mar 2015 18:59:38 -0600 Subject: [PATCH 03/14] zeros are ok after all --- influxql/parser.go | 34 +++++++++++++++++----------------- influxql/parser_test.go | 2 -- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/influxql/parser.go b/influxql/parser.go index dfc8ace39b..6aacaa407a 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -564,22 +564,22 @@ func (p *Parser) parseSelectStatement(tr targetRequirement) (*SelectStatement, e } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } // Parse series limit: "SLIMIT ". - if stmt.SLimit, err = p.parseOptionalTokenAndInt(SLIMIT, false); err != nil { + if stmt.SLimit, err = p.parseOptionalTokenAndInt(SLIMIT); err != nil { return nil, err } // Parse series offset: "SOFFSET ". - if stmt.SOffset, err = p.parseOptionalTokenAndInt(SOFFSET, false); err != nil { + if stmt.SOffset, err = p.parseOptionalTokenAndInt(SOFFSET); err != nil { return nil, err } @@ -679,12 +679,12 @@ func (p *Parser) parseShowSeriesStatement() (*ShowSeriesStatement, error) { } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, true); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } @@ -708,12 +708,12 @@ func (p *Parser) parseShowMeasurementsStatement() (*ShowMeasurementsStatement, e } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } @@ -760,12 +760,12 @@ func (p *Parser) parseShowTagKeysStatement() (*ShowTagKeysStatement, error) { } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } @@ -803,12 +803,12 @@ func (p *Parser) parseShowTagValuesStatement() (*ShowTagValuesStatement, error) } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } @@ -883,12 +883,12 @@ func (p *Parser) parseShowFieldKeysStatement() (*ShowFieldKeysStatement, error) } // Parse limit: "LIMIT ". - if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT, false); err != nil { + if stmt.Limit, err = p.parseOptionalTokenAndInt(LIMIT); err != nil { return nil, err } // Parse offset: "OFFSET ". - if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET, false); err != nil { + if stmt.Offset, err = p.parseOptionalTokenAndInt(OFFSET); err != nil { return nil, err } @@ -1413,7 +1413,7 @@ func (p *Parser) parseFill() (FillOption, interface{}, error) { // parseOptionalTokenAndInt parses the specified token followed // by an int, if it exists. -func (p *Parser) parseOptionalTokenAndInt(t Token, allowZero bool) (int, error) { +func (p *Parser) parseOptionalTokenAndInt(t Token) (int, error) { // Check if the token exists. if tok, _, _ := p.scanIgnoreWhitespace(); tok != t { p.unscan() @@ -1435,8 +1435,8 @@ func (p *Parser) parseOptionalTokenAndInt(t Token, allowZero bool) (int, error) // Parse number. n, _ := strconv.ParseInt(lit, 10, 64) - if n < 1 && !allowZero { - msg := fmt.Sprintf("%s must be > 0", t.String()) + if n < 0 { + msg := fmt.Sprintf("%s must be >= 0", t.String()) return 0, &ParseError{Message: msg, Pos: pos} } diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 5cb6b6520b..00a2606069 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -778,10 +778,8 @@ func TestParser_ParseStatement(t *testing.T) { {s: `SELECT field1 FROM myseries GROUP`, err: `found EOF, expected BY at line 1, char 35`}, {s: `SELECT field1 FROM myseries LIMIT`, err: `found EOF, expected number at line 1, char 35`}, {s: `SELECT field1 FROM myseries LIMIT 10.5`, err: `fractional parts not allowed in LIMIT at line 1, char 35`}, - {s: `SELECT field1 FROM myseries LIMIT 0`, err: `LIMIT must be > 0 at line 1, char 35`}, {s: `SELECT field1 FROM myseries OFFSET`, err: `found EOF, expected number at line 1, char 36`}, {s: `SELECT field1 FROM myseries OFFSET 10.5`, err: `fractional parts not allowed in OFFSET at line 1, char 36`}, - {s: `SELECT field1 FROM myseries OFFSET 0`, err: `OFFSET must be > 0 at line 1, char 36`}, {s: `SELECT field1 FROM myseries ORDER`, err: `found EOF, expected BY at line 1, char 35`}, {s: `SELECT field1 FROM myseries ORDER BY /`, err: `found /, expected identifier, ASC, or DESC at line 1, char 38`}, {s: `SELECT field1 FROM myseries ORDER BY 1`, err: `found 1, expected identifier, ASC, or DESC at line 1, char 38`}, From 55c0e4434f60dd64516e35ae266c055356ca1aa4 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Fri, 13 Mar 2015 07:31:24 -0600 Subject: [PATCH 04/14] Update RC11 Date for Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2468d8ae02..103ff02163 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## v0.9.0-rc11 [unreleased] +## v0.9.0-rc11 [2015-03-13] ### Bugfixes - [#1917](https://github.com/influxdb/influxdb/pull/1902): Creating Infinite Retention Policy Failed. From 932b6ddc81bc6fbe6eb1787afda3c6633650ec5e Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Fri, 13 Mar 2015 16:51:18 -0600 Subject: [PATCH 05/14] Sort wildcard expresion field names for consistent output --- influxql/ast.go | 8 ++++++++ server_test.go | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/influxql/ast.go b/influxql/ast.go index 8d69f9922d..4b198209fb 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "regexp" + "sort" "strconv" "strings" "time" @@ -654,6 +655,8 @@ func (s *SelectStatement) RewriteWildcards(fields Fields, dimensions Dimensions) for _, f := range s.Fields { switch f.Expr.(type) { case *Wildcard: + // Sort wildcard fields for consistent output + sort.Sort(fields) rwFields = append(rwFields, fields...) default: rwFields = append(rwFields, f) @@ -1573,6 +1576,11 @@ func (f *Field) String() string { return fmt.Sprintf("%s AS %s", f.Expr.String(), f.Alias) } +// Sort Interface for Field +func (f Fields) Len() int { return len(f) } +func (f Fields) Less(i, j int) bool { return f[i].Name() < f[j].Name() } +func (f Fields) Swap(i, j int) { f[i], f[j] = f[j], f[i] } + // Dimensions represents a list of dimensions. type Dimensions []*Dimension diff --git a/server_test.go b/server_test.go index 9de930c479..69849156eb 100644 --- a/server_test.go +++ b/server_test.go @@ -1741,7 +1741,8 @@ func TestServer_ExecuteWildcardQuery(t *testing.T) { results := s.ExecuteQuery(MustParseQuery(`SELECT * FROM cpu`), "foo", nil) if res := results.Results[0]; res.Err != nil { t.Fatalf("unexpected error during SELECT *: %s", res.Err) - } else if s := mustMarshalJSON(res); s != `{"series":[{"name":"cpu","columns":["time","value","val-x"],"values":[["2000-01-01T00:00:00Z",10,null],["2000-01-01T00:00:10Z",null,20],["2000-01-01T00:00:20Z",30,40]]}]}` { + } else if s, e := mustMarshalJSON(res), `{"series":[{"name":"cpu","columns":["time","val-x","value"],"values":[["2000-01-01T00:00:00Z",null,10],["2000-01-01T00:00:10Z",20,null],["2000-01-01T00:00:20Z",40,30]]}]}`; s != e { + t.Logf("expected %s\n", e) t.Fatalf("unexpected results during SELECT *: %s", s) } } From 3c3765dbb1de0f29b757f0e5f407c1e00ca706b5 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Fri, 13 Mar 2015 16:52:49 -0600 Subject: [PATCH 06/14] typo in comment --- influxql/ast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/influxql/ast.go b/influxql/ast.go index 4b198209fb..5405e2a188 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -1576,7 +1576,7 @@ func (f *Field) String() string { return fmt.Sprintf("%s AS %s", f.Expr.String(), f.Alias) } -// Sort Interface for Field +// Sort Interface for Fields func (f Fields) Len() int { return len(f) } func (f Fields) Less(i, j int) bool { return f[i].Name() < f[j].Name() } func (f Fields) Swap(i, j int) { f[i], f[j] = f[j], f[i] } From 8ceff154fa482b2f4dfea7a41689263d90e5696b Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Fri, 13 Mar 2015 16:56:54 -0600 Subject: [PATCH 07/14] update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 103ff02163..c820967d3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v0.9.0-rc12 [unreleased] + +### Bugfixes +- [#1942](https://github.com/influxdb/influxdb/pull/1942): Sort wildcard names. + ## v0.9.0-rc11 [2015-03-13] ### Bugfixes From a37a3038520ad8d42ef76a5134c00096e901c09e Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Fri, 13 Mar 2015 10:29:58 -0600 Subject: [PATCH 08/14] added tests for writing multiple values/fields for client --- cmd/influxd/server_integration_test.go | 140 +++++++++++++++++-------- 1 file changed, 94 insertions(+), 46 deletions(-) diff --git a/cmd/influxd/server_integration_test.go b/cmd/influxd/server_integration_test.go index ea749e04f1..30acd2ed5f 100644 --- a/cmd/influxd/server_integration_test.go +++ b/cmd/influxd/server_integration_test.go @@ -866,43 +866,79 @@ func TestClientLibrary(t *testing.T) { os.RemoveAll(dir) }() - database := "mydb" - retentionPolicy := "myrp" now := time.Now().UTC() nodes := createCombinedNodeCluster(t, testName, dir, 1, 8290, nil) - createDatabase(t, testName, nodes, database) - createRetentionPolicy(t, testName, nodes, database, retentionPolicy) + type write struct { + bp client.BatchPoints + expected string + err string + } + type query struct { + query client.Query + expected string + err string + } + type test struct { + name string + db string + rp string + writes []write + queries []query + } - tests := []struct { - name string - bp client.BatchPoints - results client.Results - query client.Query - writeExpected, queryExpected string - writeErr, queryErr string - }{ + tests := []test{ { - name: "empty batchpoint", - writeErr: "database is required", - writeExpected: `{"error":"database is required"}`, + name: "empty batchpoint", + writes: []write{ + { + err: "database is required", + expected: `{"error":"database is required"}`, + }, + }, }, { - name: "no points", - writeExpected: `null`, - bp: client.BatchPoints{Database: "mydb"}, + name: "no points", + writes: []write{ + { + expected: `null`, + bp: client.BatchPoints{Database: "mydb"}, + }, + }, }, { name: "one point", - bp: client.BatchPoints{ - Database: "mydb", - Points: []client.Point{ - {Name: "cpu", Fields: map[string]interface{}{"value": 1.1}, Timestamp: now}, + writes: []write{ + { + bp: client.BatchPoints{ + Database: "mydb", + Points: []client.Point{ + {Name: "cpu", Fields: map[string]interface{}{"value": 1.1}, Timestamp: now}, + }, + }, + expected: `null`, + }, + }, + queries: []query{ + { + query: client.Query{Command: `select * from "mydb"."myrp".cpu`}, + expected: fmt.Sprintf(`{"results":[{"series":[{"name":"cpu","columns":["time","value"],"values":[["%s",1.1]]}]}]}`, now.Format(time.RFC3339Nano)), + }, + }, + }, + { + name: "mulitple points, multiple values", + writes: []write{ + {bp: client.BatchPoints{Database: "mydb", Points: []client.Point{{Name: "network", Fields: map[string]interface{}{"rx": 1.1, "tx": 2.1}, Timestamp: now}}}, expected: `null`}, + {bp: client.BatchPoints{Database: "mydb", Points: []client.Point{{Name: "network", Fields: map[string]interface{}{"rx": 1.2, "tx": 2.2}, Timestamp: now.Add(time.Nanosecond)}}}, expected: `null`}, + {bp: client.BatchPoints{Database: "mydb", Points: []client.Point{{Name: "network", Fields: map[string]interface{}{"rx": 1.3, "tx": 2.3}, Timestamp: now.Add(2 * time.Nanosecond)}}}, expected: `null`}, + }, + queries: []query{ + { + query: client.Query{Command: `select * from "mydb"."myrp".network`}, + expected: fmt.Sprintf(`{"results":[{"series":[{"name":"network","columns":["time","rx","tx"],"values":[["%s",1.1,2.1],["%s",1.2,2.2],["%s",1.3,2.3]]}]}]}`, now.Format(time.RFC3339Nano), now.Add(time.Nanosecond).Format(time.RFC3339Nano), now.Add(2*time.Nanosecond).Format(time.RFC3339Nano)), }, }, - writeExpected: `null`, - query: client.Query{Command: `select * from "mydb"."myrp".cpu`}, - queryExpected: fmt.Sprintf(`{"results":[{"series":[{"name":"cpu","columns":["time","value"],"values":[["%s",1.1]]}]}]}`, now.Format(time.RFC3339Nano)), }, } @@ -912,32 +948,44 @@ func TestClientLibrary(t *testing.T) { } for _, test := range tests { + if test.db == "" { + test.db = "mydb" + } + if test.rp == "" { + test.rp = "myrp" + } + createDatabase(t, testName, nodes, test.db) + createRetentionPolicy(t, testName, nodes, test.db, test.rp) t.Logf("testing %s - %s\n", testName, test.name) - writeResult, err := c.Write(test.bp) - if test.writeErr != errToString(err) { - t.Errorf("unexpected error. expected: %s, got %v", test.writeErr, err) - } - jsonResult := mustMarshalJSON(writeResult) - if test.writeExpected != jsonResult { - t.Logf("write expected result: %s\n", test.writeExpected) - t.Logf("write got result: %s\n", jsonResult) - t.Error("unexpected results") - } - - if test.query.Command != "" { - time.Sleep(100 * time.Millisecond) - queryResult, err := c.Query(test.query) - if test.queryErr != errToString(err) { - t.Errorf("unexpected error. expected: %s, got %v", test.queryErr, err) + for _, w := range test.writes { + writeResult, err := c.Write(w.bp) + if w.err != errToString(err) { + t.Errorf("unexpected error. expected: %s, got %v", w.err, err) } - jsonResult := mustMarshalJSON(queryResult) - if test.queryExpected != jsonResult { - t.Logf("query expected result: %s\n", test.queryExpected) - t.Logf("query got result: %s\n", jsonResult) + jsonResult := mustMarshalJSON(writeResult) + if w.expected != jsonResult { + t.Logf("write expected result: %s\n", w.expected) + t.Logf("write got result: %s\n", jsonResult) t.Error("unexpected results") } - } + + for _, q := range test.queries { + if q.query.Command != "" { + time.Sleep(100 * time.Millisecond) + queryResult, err := c.Query(q.query) + if q.err != errToString(err) { + t.Errorf("unexpected error. expected: %s, got %v", q.err, err) + } + jsonResult := mustMarshalJSON(queryResult) + if q.expected != jsonResult { + t.Logf("query expected result: %s\n", q.expected) + t.Logf("query got result: %s\n", jsonResult) + t.Error("unexpected results") + } + } + } + deleteDatabase(t, testName, nodes, test.db) } } From 38bb2cf62e4c4a974d32e7ea5fe9f383994f5937 Mon Sep 17 00:00:00 2001 From: Giulio Iotti Date: Sat, 14 Mar 2015 15:34:55 +0200 Subject: [PATCH 09/14] do not allow empty database name, closes #1950 --- database.go | 4 ++++ server.go | 3 +++ server_test.go | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/database.go b/database.go index 1a03cc634c..60e63c05da 100644 --- a/database.go +++ b/database.go @@ -81,6 +81,10 @@ func (db *database) UnmarshalJSON(data []byte) error { return err } + if o.Name == "" { + return ErrDatabaseNameRequired + } + // Copy over properties from intermediate type. db.name = o.Name db.defaultRetentionPolicy = o.DefaultRetentionPolicy diff --git a/server.go b/server.go index f8f9d03dff..f5facd8e44 100644 --- a/server.go +++ b/server.go @@ -747,6 +747,9 @@ func (s *Server) Databases() (a []string) { // CreateDatabase creates a new database. func (s *Server) CreateDatabase(name string) error { + if name == "" { + return ErrDatabaseNameRequired + } c := &createDatabaseCommand{Name: name} _, err := s.broadcast(createDatabaseMessageType, c) return err diff --git a/server_test.go b/server_test.go index 69849156eb..eea7638fea 100644 --- a/server_test.go +++ b/server_test.go @@ -290,6 +290,11 @@ func TestServer_CreateDatabase(t *testing.T) { s := OpenServer(NewMessagingClient()) defer s.Close() + // Attempt creating empty name database. + if err := s.CreateDatabase(""); err != influxdb.ErrDatabaseNameRequired { + t.Fatal("expected error on empty database name") + } + // Create the "foo" database. if err := s.CreateDatabase("foo"); err != nil { t.Fatal(err) From a19f81d3e979633d829191c23da8f345b58e2aa9 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Sat, 14 Mar 2015 09:59:33 -0600 Subject: [PATCH 10/14] Graphite numbers are always float64 --- cmd/influxd/server_integration_test.go | 53 +++++++++++++++++++++++++- graphite/graphite.go | 7 +--- graphite/graphite_test.go | 50 ++++++++---------------- 3 files changed, 68 insertions(+), 42 deletions(-) diff --git a/cmd/influxd/server_integration_test.go b/cmd/influxd/server_integration_test.go index 30acd2ed5f..3624e7c01a 100644 --- a/cmd/influxd/server_integration_test.go +++ b/cmd/influxd/server_integration_test.go @@ -1039,7 +1039,7 @@ func Test_ServerSingleGraphiteIntegration(t *testing.T) { } } -func Test_ServerSingleGraphiteIntegration_NoDatabase(t *testing.T) { +func Test_ServerSingleGraphiteIntegration_ZeroDataPoint(t *testing.T) { if testing.Short() { t.Skip() } @@ -1051,7 +1051,58 @@ func Test_ServerSingleGraphiteIntegration_NoDatabase(t *testing.T) { c := main.NewConfig() g := main.Graphite{ Enabled: true, + Database: "graphite", + Protocol: "TCP", Port: 2103, + } + c.Graphites = append(c.Graphites, g) + + t.Logf("Graphite Connection String: %s\n", g.ConnectionString(c.BindAddress)) + nodes := createCombinedNodeCluster(t, testName, dir, nNodes, basePort, c) + + createDatabase(t, testName, nodes, "graphite") + createRetentionPolicy(t, testName, nodes, "graphite", "raw") + + // Connect to the graphite endpoint we just spun up + conn, err := net.Dial("tcp", g.ConnectionString(c.BindAddress)) + if err != nil { + t.Fatal(err) + return + } + + t.Log("Writing data") + data := []byte(`cpu 0.000 `) + data = append(data, []byte(fmt.Sprintf("%d", now.UnixNano()/1000000))...) + data = append(data, '\n') + _, err = conn.Write(data) + conn.Close() + if err != nil { + t.Fatal(err) + return + } + + expected := fmt.Sprintf(`{"results":[{"series":[{"name":"cpu","columns":["time","cpu"],"values":[["%s",0]]}]}]}`, now.Format(time.RFC3339Nano)) + + // query and wait for results + got, ok := queryAndWait(t, nodes, "graphite", `select * from "graphite"."raw".cpu`, expected, 2*time.Second) + if !ok { + t.Errorf(`Test "%s" failed, expected: %s, got: %s`, testName, expected, got) + } +} + +func Test_ServerSingleGraphiteIntegration_NoDatabase(t *testing.T) { + if testing.Short() { + t.Skip() + } + nNodes := 1 + basePort := 8590 + testName := "graphite integration" + dir := tempfile() + now := time.Now().UTC().Round(time.Millisecond) + c := main.NewConfig() + g := main.Graphite{ + Enabled: true, + Port: 2203, Protocol: "TCP", } c.Graphites = append(c.Graphites, g) diff --git a/graphite/graphite.go b/graphite/graphite.go index 1de9e7ce94..3e10c6e344 100644 --- a/graphite/graphite.go +++ b/graphite/graphite.go @@ -86,12 +86,7 @@ func (p *Parser) Parse(line string) (influxdb.Point, error) { } fieldValues := make(map[string]interface{}) - // Determine if value is a float or an int. - if i := int64(v); float64(i) == v { - fieldValues[name] = int64(v) - } else { - fieldValues[name] = v - } + fieldValues[name] = v // Parse timestamp. unixTime, err := strconv.ParseInt(fields[2], 10, 64) diff --git a/graphite/graphite_test.go b/graphite/graphite_test.go index 798a95d074..ffc0231bfd 100644 --- a/graphite/graphite_test.go +++ b/graphite/graphite_test.go @@ -61,9 +61,7 @@ func Test_DecodeMetric(t *testing.T) { line string name string tags map[string]string - isInt bool - iv int64 - fv float64 + value float64 timestamp time.Time position, separator string err string @@ -73,8 +71,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu.foo.bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -83,8 +80,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu.foo.bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -93,8 +89,7 @@ func Test_DecodeMetric(t *testing.T) { line: `foo.bar.cpu 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -103,8 +98,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu 50 ` + strTime, name: "cpu", tags: map[string]string{}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -113,8 +107,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu 50 ` + strTime, name: "cpu", tags: map[string]string{}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -122,8 +115,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu.foo.bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -132,8 +124,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu.foo.bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -142,8 +133,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu-foo-bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { @@ -152,8 +142,7 @@ func Test_DecodeMetric(t *testing.T) { line: `cpuboofooboobar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, @@ -162,16 +151,14 @@ func Test_DecodeMetric(t *testing.T) { line: `cpu.foo.bar 50 ` + strTime, name: "cpu", tags: map[string]string{"foo": "bar"}, - isInt: true, - iv: 50, + value: 50, timestamp: testTime, }, { test: "metric only with float value", line: `cpu 50.554 ` + strTime, name: "cpu", - isInt: false, - fv: 50.554, + value: 50.554, timestamp: testTime, }, { @@ -224,16 +211,9 @@ func Test_DecodeMetric(t *testing.T) { if len(point.Tags) != len(test.tags) { t.Fatalf("tags len mismatch. expected %d, got %d", len(test.tags), len(point.Tags)) } - if test.isInt { - i := point.Fields[point.Name].(int64) - if i != test.iv { - t.Fatalf("integerValue value mismatch. expected %v, got %v", test.iv, point.Fields[point.Name]) - } - } else { - f := point.Fields[point.Name].(float64) - if point.Fields[point.Name] != f { - t.Fatalf("floatValue value mismatch. expected %v, got %v", test.fv, f) - } + f := point.Fields[point.Name].(float64) + if point.Fields[point.Name] != f { + t.Fatalf("floatValue value mismatch. expected %v, got %v", test.value, f) } if point.Timestamp.UnixNano()/1000000 != test.timestamp.UnixNano()/1000000 { t.Fatalf("timestamp value mismatch. expected %v, got %v", test.timestamp.UnixNano(), point.Timestamp.UnixNano()) From c0d3220a021bd4ed51a28958ac67562e74709494 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Sat, 14 Mar 2015 10:03:09 -0600 Subject: [PATCH 11/14] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c820967d3e..1ac5902c5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Bugfixes - [#1942](https://github.com/influxdb/influxdb/pull/1942): Sort wildcard names. +- [#1957](https://github.com/influxdb/influxdb/pull/1957): Graphite numbers are always float64. ## v0.9.0-rc11 [2015-03-13] From 2cc2077c3d75937352bc2c67ea01a29b0f7d9210 Mon Sep 17 00:00:00 2001 From: Giulio Iotti Date: Sat, 14 Mar 2015 21:31:44 +0200 Subject: [PATCH 12/14] do not accept empty database name in drop --- database.go | 4 ---- server.go | 3 +++ server_test.go | 5 +++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/database.go b/database.go index 60e63c05da..1a03cc634c 100644 --- a/database.go +++ b/database.go @@ -81,10 +81,6 @@ func (db *database) UnmarshalJSON(data []byte) error { return err } - if o.Name == "" { - return ErrDatabaseNameRequired - } - // Copy over properties from intermediate type. db.name = o.Name db.defaultRetentionPolicy = o.DefaultRetentionPolicy diff --git a/server.go b/server.go index f5facd8e44..5cb75a0335 100644 --- a/server.go +++ b/server.go @@ -798,6 +798,9 @@ func (s *Server) applyCreateDatabase(m *messaging.Message) (err error) { // DropDatabase deletes an existing database. func (s *Server) DropDatabase(name string) error { + if name == "" { + return ErrDatabaseNameRequired + } c := &dropDatabaseCommand{Name: name} _, err := s.broadcast(dropDatabaseMessageType, c) return err diff --git a/server_test.go b/server_test.go index eea7638fea..e834faed7c 100644 --- a/server_test.go +++ b/server_test.go @@ -326,6 +326,11 @@ func TestServer_DropDatabase(t *testing.T) { s := OpenServer(NewMessagingClient()) defer s.Close() + // Attempt creating empty name database. + if err := s.DropDatabase(""); err != influxdb.ErrDatabaseNameRequired { + t.Fatal("expected error on empty database name") + } + // Create the "foo" database and verify it exists. if err := s.CreateDatabase("foo"); err != nil { t.Fatal(err) From 9989a6a400063627d557cb848d394037f604f82d Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 14 Mar 2015 12:38:00 -0700 Subject: [PATCH 13/14] Tweak comments for database creation tests --- server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server_test.go b/server_test.go index e834faed7c..62aafd6402 100644 --- a/server_test.go +++ b/server_test.go @@ -290,7 +290,7 @@ func TestServer_CreateDatabase(t *testing.T) { s := OpenServer(NewMessagingClient()) defer s.Close() - // Attempt creating empty name database. + // Attempt creating database without a name if err := s.CreateDatabase(""); err != influxdb.ErrDatabaseNameRequired { t.Fatal("expected error on empty database name") } @@ -326,7 +326,7 @@ func TestServer_DropDatabase(t *testing.T) { s := OpenServer(NewMessagingClient()) defer s.Close() - // Attempt creating empty name database. + // Attempt dropping a database without a name. if err := s.DropDatabase(""); err != influxdb.ErrDatabaseNameRequired { t.Fatal("expected error on empty database name") } From 11a908fbbec9aa77ca4b9d7de6554dcdd841af58 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sat, 14 Mar 2015 12:39:42 -0700 Subject: [PATCH 14/14] Update CHANGELOG for PR 1955 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ac5902c5f..750873e0fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Bugfixes - [#1942](https://github.com/influxdb/influxdb/pull/1942): Sort wildcard names. - [#1957](https://github.com/influxdb/influxdb/pull/1957): Graphite numbers are always float64. +- [#1955](https://github.com/influxdb/influxdb/pull/1955): Prohibit creation of databases with no name. Thanks @dullgiulio ## v0.9.0-rc11 [2015-03-13]