Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add uint support into the write protocol #8835

Merged
merged 1 commit into from
Oct 9, 2017
Merged

Add uint support into the write protocol #8835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions models/points.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ const (
MaxKeyLength = 65535
)

// enableUint64Support will enable uint64 support if set to true.
var enableUint64Support = false

// EnableUintSupport manually enables uint support for the point parser.
// This function will be removed in the future and only exists for unit tests during the
// transition.
func EnableUintSupport() {
enableUint64Support = true
}

// Point defines the values that will be written to the database.
type Point interface {
// Name return the measurement name for the point.
Expand Down Expand Up @@ -224,6 +234,9 @@ const (
// the number of characters for the smallest possible int64 (-9223372036854775808)
minInt64Digits = 20

// the number of characters for the largest possible uint64 (18446744073709551615)
maxUint64Digits = 20

// the number of characters required for the largest float64 before a range check
// would occur during parsing
maxFloat64Digits = 25
Expand Down Expand Up @@ -827,7 +840,7 @@ func isNumeric(b byte) bool {
// error if a invalid number is scanned.
func scanNumber(buf []byte, i int) (int, error) {
start := i
var isInt bool
var isInt, isUnsigned bool

// Is negative number?
if i < len(buf) && buf[i] == '-' {
Expand All @@ -853,10 +866,14 @@ func scanNumber(buf []byte, i int) (int, error) {
break
}

if buf[i] == 'i' && i > start && !isInt {
if buf[i] == 'i' && i > start && !(isInt || isUnsigned) {
isInt = true
i++
continue
} else if buf[i] == 'u' && i > start && !(isInt || isUnsigned) {
isUnsigned = true
i++
continue
}

if buf[i] == '.' {
Expand Down Expand Up @@ -891,7 +908,7 @@ func scanNumber(buf []byte, i int) (int, error) {
i++
}

if isInt && (decimal || scientific) {
if (isInt || isUnsigned) && (decimal || scientific) {
return i, ErrInvalidNumber
}

Expand Down Expand Up @@ -926,6 +943,26 @@ func scanNumber(buf []byte, i int) (int, error) {
return i, fmt.Errorf("unable to parse integer %s: %s", buf[start:i-1], err)
}
}
} else if isUnsigned {
// Return an error if uint64 support has not been enabled.
if !enableUint64Support {
return i, ErrInvalidNumber
}
// Make sure the last char is a 'u' for unsigned
if buf[i-1] != 'u' {
return i, ErrInvalidNumber
}
// Make sure the first char is not a '-' for unsigned
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is redundant with the initial check at line 833?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so. The check at line 833 prevents a person from writing m=-. This checks to make sure a negative wasn't used while looking at an unsigned integer.

if buf[start] == '-' {
return i, ErrInvalidNumber
}
// Parse the uint to check bounds the number of digits could be larger than the max range
// We subtract 1 from the index to remove the `u` from our tests
if len(buf[start:i-1]) >= maxUint64Digits {
if _, err := parseUintBytes(buf[start:i-1], 10, 64); err != nil {
return i, fmt.Errorf("unable to parse unsigned %s: %s", buf[start:i-1], err)
}
}
} else {
// Parse the float to check bounds if it's scientific or the number of digits could be larger than the max range
if scientific || len(buf[start:i]) >= maxFloat64Digits || len(buf[start:i]) >= minFloat64Digits {
Expand Down Expand Up @@ -2118,10 +2155,13 @@ func (p *point) Next() bool {
return true
}

if strings.IndexByte(`0123456789-.nNiI`, c) >= 0 {
if strings.IndexByte(`0123456789-.nNiIu`, c) >= 0 {
if p.it.valueBuf[len(p.it.valueBuf)-1] == 'i' {
p.it.fieldType = Integer
p.it.valueBuf = p.it.valueBuf[:len(p.it.valueBuf)-1]
} else if p.it.valueBuf[len(p.it.valueBuf)-1] == 'u' {
p.it.fieldType = Unsigned
p.it.valueBuf = p.it.valueBuf[:len(p.it.valueBuf)-1]
} else {
p.it.fieldType = Float
}
Expand Down
92 changes: 88 additions & 4 deletions models/points_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ func TestParsePointBadNumber(t *testing.T) {
"cpu v=-e-e-e ",
"cpu v=42+3 ",
"cpu v= ",
"cpu v=-123u",
} {
_, err := models.ParsePointsString(tt)
if err == nil {
Expand Down Expand Up @@ -567,10 +568,17 @@ func TestParsePointMinInt64(t *testing.T) {
}

// min int
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=-9223372036854775808i`)
p, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=-9223372036854775808i`)
if err != nil {
t.Errorf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=-9223372036854775808i`, err)
}
fields, err := p[0].Fields()
if err != nil {
t.Fatal(err)
}
if exp, got := int64(-9223372036854775808), fields["value"].(int64); exp != got {
t.Fatalf("ParsePoints Value mismatch. \nexp: %v\ngot: %v", exp, got)
}

// leading zeros
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=-0009223372036854775808i`)
Expand All @@ -587,10 +595,17 @@ func TestParsePointMaxFloat64(t *testing.T) {
}

// max float
_, err = models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, string(maxFloat64)))
p, err := models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, string(maxFloat64)))
if err != nil {
t.Errorf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=9223372036854775807`, err)
}
fields, err := p[0].Fields()
if err != nil {
t.Fatal(err)
}
if exp, got := math.MaxFloat64, fields["value"].(float64); exp != got {
t.Fatalf("ParsePoints Value mismatch. \nexp: %v\ngot: %v", exp, got)
}

// leading zeros
_, err = models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, "0000"+string(maxFloat64)))
Expand All @@ -607,10 +622,17 @@ func TestParsePointMinFloat64(t *testing.T) {
}

// min float
_, err = models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, string(minFloat64)))
p, err := models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, string(minFloat64)))
if err != nil {
t.Errorf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=...`, err)
}
fields, err := p[0].Fields()
if err != nil {
t.Fatal(err)
}
if exp, got := -math.MaxFloat64, fields["value"].(float64); exp != got {
t.Fatalf("ParsePoints Value mismatch. \nexp: %v\ngot: %v", exp, got)
}

// leading zeros
_, err = models.ParsePointsString(fmt.Sprintf(`cpu,host=serverA,region=us-west value=%s`, "-0000000"+string(minFloat64)[1:]))
Expand All @@ -619,6 +641,61 @@ func TestParsePointMinFloat64(t *testing.T) {
}
}

func TestParsePointMaxUint64(t *testing.T) {
// out of range
_, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=18446744073709551616u`)
exp := `unable to parse 'cpu,host=serverA,region=us-west value=18446744073709551616u': unable to parse unsigned 18446744073709551616: strconv.ParseUint: parsing "18446744073709551616": value out of range`
if err == nil || (err != nil && err.Error() != exp) {
t.Fatalf("Error mismatch:\nexp: %s\ngot: %v", exp, err)
}

// max int
p, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=18446744073709551615u`)
if err != nil {
t.Fatalf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=18446744073709551615u`, err)
}
fields, err := p[0].Fields()
if err != nil {
t.Fatal(err)
}
if exp, got := uint64(18446744073709551615), fields["value"].(uint64); exp != got {
t.Fatalf("ParsePoints Value mismatch. \nexp: %v\ngot: %v", exp, got)
}

// leading zeros
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=00018446744073709551615u`)
if err != nil {
t.Fatalf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=00018446744073709551615u`, err)
}
}

func TestParsePointMinUint64(t *testing.T) {
// out of range
_, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=--1u`)
if err == nil {
t.Errorf(`ParsePoints("%s") mismatch. got nil, exp error`, `cpu,host=serverA,region=us-west value=-1u`)
}

// min int
p, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=0u`)
if err != nil {
t.Errorf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=0u`, err)
}
fields, err := p[0].Fields()
if err != nil {
t.Fatal(err)
}
if exp, got := uint64(0), fields["value"].(uint64); exp != got {
t.Fatalf("ParsePoints Value mismatch. \nexp: %v\ngot: %v", exp, got)
}

// leading zeros
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=0000u`)
if err != nil {
t.Errorf(`ParsePoints("%s") mismatch. got %v, exp nil`, `cpu,host=serverA,region=us-west value=0000u`, err)
}
}

func TestParsePointNumberNonNumeric(t *testing.T) {
_, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=.1a`)
if err == nil {
Expand Down Expand Up @@ -2148,6 +2225,8 @@ func toFields(fi models.FieldIterator) models.Fields {
v, err = fi.FloatValue()
case models.Integer:
v, err = fi.IntegerValue()
case models.Unsigned:
v, err = fi.UnsignedValue()
case models.String:
v = fi.StringValue()
case models.Boolean:
Expand All @@ -2173,7 +2252,7 @@ m v=42i
m v="string"
m v=true
m v="string\"with\"escapes"
m v=42i,f=42,g=42.314
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be reading the test incompletely - but need to be sure we have tests for a negative unsigned int (error path), for max value, and for something greater than the max value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add those.

m v=42i,f=42,g=42.314,u=123u
m a=2i,b=3i,c=true,d="stuff",e=-0.23,f=123.456
`)

Expand Down Expand Up @@ -2251,3 +2330,8 @@ func BenchmarkEscapeString_QuotesAndBackslashes(b *testing.B) {
sink = [...]string{models.EscapeStringField(s1), models.EscapeStringField(s2)}
}
}

func init() {
// Force uint support to be enabled for testing.
models.EnableUintSupport()
}
7 changes: 7 additions & 0 deletions models/uint_support.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// +build uint uint64

package models

func init() {
EnableUintSupport()
}