Skip to content

Commit

Permalink
buildtsi: Do not escape measurement names
Browse files Browse the repository at this point in the history
When `influx_inspect buildtsi` is used to create a new `tsi1` index, spaces in measurement names are escaped, so measurement "a b" is changed to "a\ b".

This change modifies `models.ParseKeyBytes()` and `models.ParseName()` to unescape measurement names. `models.ParseKeyBytes()` returns unescaped tag keys, so this seems like the natural place to unescape measurement names.

Also followed `scanMeasurement()` to see what other code could be problematic, and this should be everything (the result of one other use of `scanMeasurement()` is later escaped).

Removed `tsdb.MeasurementFromSeriesKey()`. These methods are exported, so checked for side effects in other InfluxData repositories.
  • Loading branch information
Jacob Marble committed May 30, 2018
1 parent 0253f6f commit aac70ba
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 17 deletions.
21 changes: 14 additions & 7 deletions models/points.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,36 +273,43 @@ func ParsePointsString(buf string) ([]Point, error) {
// NOTE: to minimize heap allocations, the returned Tags will refer to subslices of buf.
// This can have the unintended effect preventing buf from being garbage collected.
func ParseKey(buf []byte) (string, Tags) {
meas, tags := ParseKeyBytes(buf)
return string(meas), tags
name, tags := ParseKeyBytes(buf)
return string(name), tags
}

func ParseKeyBytes(buf []byte) ([]byte, Tags) {
// Ignore the error because scanMeasurement returns "missing fields" which we ignore
// when just parsing a key
state, i, _ := scanMeasurement(buf, 0)

var name []byte
var tags Tags
if state == tagKeyState {
tags = parseTags(buf)
// scanMeasurement returns the location of the comma if there are tags, strip that off
return buf[:i-1], tags
name = buf[:i-1]
} else {
name = buf[:i]
}
return buf[:i], tags
return unescapeMeasurement(name), tags
}

func ParseTags(buf []byte) Tags {
return parseTags(buf)
}

func ParseName(buf []byte) ([]byte, error) {
func ParseName(buf []byte) []byte {
// Ignore the error because scanMeasurement returns "missing fields" which we ignore
// when just parsing a key
state, i, _ := scanMeasurement(buf, 0)
var name []byte
if state == tagKeyState {
return buf[:i-1], nil
name = buf[:i-1]
} else {
name = buf[:i]
}
return buf[:i], nil

return unescapeMeasurement(name)
}

// ParsePointsWithPrecision is similar to ParsePoints, but allows the
Expand Down
44 changes: 44 additions & 0 deletions models/points_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2388,6 +2388,50 @@ func TestEscapeStringField(t *testing.T) {
}
}

func TestParseKeyBytes(t *testing.T) {
testCases := []struct {
input string
expectedName string
expectedTags map[string]string
}{
{input: "m,k=v", expectedName: "m", expectedTags: map[string]string{"k": "v"}},
{input: "m\\ q,k=v", expectedName: "m q", expectedTags: map[string]string{"k": "v"}},
{input: "m,k\\ q=v", expectedName: "m", expectedTags: map[string]string{"k q": "v"}},
{input: "m\\ q,k\\ q=v", expectedName: "m q", expectedTags: map[string]string{"k q": "v"}},
}

for _, testCase := range testCases {
t.Run(testCase.input, func(t *testing.T) {
name, tags := models.ParseKeyBytes([]byte(testCase.input))
if !bytes.Equal([]byte(testCase.expectedName), name) {
t.Errorf("%s produced measurement %s but expected %s", testCase.input, string(name), testCase.expectedName)
}
if !tags.Equal(models.NewTags(testCase.expectedTags)) {
t.Errorf("%s produced tags %s but expected %s", testCase.input, tags.String(), models.NewTags(testCase.expectedTags).String())
}
})
}
}

func TestParseName(t *testing.T) {
testCases := []struct {
input string
expectedName string
}{
{input: "m,k=v", expectedName: "m"},
{input: "m\\ q,k=v", expectedName: "m q"},
}

for _, testCase := range testCases {
t.Run(testCase.input, func(t *testing.T) {
name := models.ParseName([]byte(testCase.input))
if !bytes.Equal([]byte(testCase.expectedName), name) {
t.Errorf("%s produced measurement %s but expected %s", testCase.input, string(name), testCase.expectedName)
}
})
}
}

func BenchmarkEscapeStringField_Plain(b *testing.B) {
s := "nothing special"
for i := 0; i < b.N; i++ {
Expand Down
2 changes: 1 addition & 1 deletion tsdb/engine/tsm1/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ func (e *Engine) addToIndexFromKey(keys [][]byte, fieldTypes []influxql.DataType
for i := 0; i < len(keys); i++ {
// Replace tsm key format with index key format.
keys[i], field = SeriesAndFieldFromCompositeKey(keys[i])
name := tsdb.MeasurementFromSeriesKey(keys[i])
name := models.ParseName(keys[i])
mf := e.fieldset.CreateFieldsIfNotExists(name)
if err := mf.CreateFieldIfNotExists(field, fieldTypes[i]); err != nil {
return err
Expand Down
9 changes: 0 additions & 9 deletions tsdb/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"sort"

"github.com/influxdata/influxdb/models"
"github.com/influxdata/influxdb/pkg/escape"
)

// MarshalTags converts a tag set to bytes for use as a lookup key.
Expand Down Expand Up @@ -97,11 +96,3 @@ func MakeTagsKey(keys []string, tags models.Tags) []byte {

return b
}

// MeasurementFromSeriesKey returns the name of the measurement from a key that
// contains a measurement name.
func MeasurementFromSeriesKey(key []byte) []byte {
// Ignoring the error because the func returns "missing fields"
k, _ := models.ParseName(key)
return escape.Unescape(k)
}

0 comments on commit aac70ba

Please sign in to comment.