diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c1f5b8e29..032bfae33a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,10 +47,13 @@ ### Bugfixes +- [#8190](https://github.com/influxdata/influxdb/issues/8190): History file should redact passwords before saving to history. +- [#8187](https://github.com/influxdata/influxdb/pull/8187): Several statements were missing the DefaultDatabase method - [#8022](https://github.com/influxdata/influxdb/issues/8022): Segment violation in models.Tags.Get - [#8155](https://github.com/influxdata/influxdb/pull/8155): Simplify admin user check. - [#8167](https://github.com/influxdata/influxdb/issues/8167): Fix a regression when math was used with selectors. - [#8175](https://github.com/influxdata/influxdb/issues/8175): Ensure the input for certain functions in the query engine are ordered. +- [#8254](https://github.com/influxdata/influxdb/pull/8254): Fix delete time fields creating unparseable points ## v1.2.2 [2017-03-14] diff --git a/models/points.go b/models/points.go index 8e9697fc3f..bee6691935 100644 --- a/models/points.go +++ b/models/points.go @@ -159,9 +159,6 @@ type FieldIterator interface { // FloatValue returns the float value of the current field. FloatValue() (float64, error) - // Delete deletes the current field. - Delete() - // Reset resets the iterator to its initial state. Reset() } @@ -2051,26 +2048,6 @@ func (p *point) FloatValue() (float64, error) { return f, nil } -// Delete deletes the current field. -func (p *point) Delete() { - switch { - case p.it.end == p.it.start: - case p.it.end >= len(p.fields): - // Remove the trailing comma if there are more than one fields - p.fields = bytes.TrimSuffix(p.fields[:p.it.start], []byte(",")) - - case p.it.start == 0: - p.fields = p.fields[p.it.end:] - default: - p.fields = append(p.fields[:p.it.start], p.fields[p.it.end:]...) - } - - p.it.end = p.it.start - p.it.key = nil - p.it.valueBuf = nil - p.it.fieldType = Empty -} - // Reset resets the iterator to its initial state. func (p *point) Reset() { p.it.fieldType = Empty diff --git a/models/points_test.go b/models/points_test.go index 1b67f2bac3..f44e7bc75a 100644 --- a/models/points_test.go +++ b/models/points_test.go @@ -2183,130 +2183,6 @@ m a=2i,b=3i,c=true,d="stuff",e=-0.23,f=123.456 } } -func TestPoint_FieldIterator_Delete_Begin(t *testing.T) { - points, err := models.ParsePointsString(`m a=1,b=2,c=3`) - if err != nil || len(points) != 1 { - t.Fatal("failed parsing point") - } - - fi := points[0].FieldIterator() - fi.Next() // a - fi.Delete() - - fi.Reset() - - got := toFields(fi) - exp := models.Fields{"b": float64(2), "c": float64(3)} - - if !reflect.DeepEqual(got, exp) { - t.Fatalf("Delete failed, got %#v, exp %#v", got, exp) - } - - if _, err = models.ParsePointsString(points[0].String()); err != nil { - t.Fatalf("Failed to parse point: %v", err) - } -} - -func TestPoint_FieldIterator_Delete_Middle(t *testing.T) { - points, err := models.ParsePointsString(`m a=1,b=2,c=3`) - if err != nil || len(points) != 1 { - t.Fatal("failed parsing point") - } - - fi := points[0].FieldIterator() - fi.Next() // a - fi.Next() // b - fi.Delete() - - fi.Reset() - - got := toFields(fi) - exp := models.Fields{"a": float64(1), "c": float64(3)} - - if !reflect.DeepEqual(got, exp) { - t.Fatalf("Delete failed, got %#v, exp %#v", got, exp) - } - - if _, err = models.ParsePointsString(points[0].String()); err != nil { - t.Fatalf("Failed to parse point: %v", err) - } -} - -func TestPoint_FieldIterator_Delete_End(t *testing.T) { - points, err := models.ParsePointsString(`m a=1,b=2,c=3`) - if err != nil || len(points) != 1 { - t.Fatal("failed parsing point") - } - - fi := points[0].FieldIterator() - fi.Next() // a - fi.Next() // b - fi.Next() // c - fi.Delete() - - fi.Reset() - - got := toFields(fi) - exp := models.Fields{"a": float64(1), "b": float64(2)} - - if !reflect.DeepEqual(got, exp) { - t.Fatalf("Delete failed, got %#v, exp %#v", got, exp) - } - - if _, err = models.ParsePointsString(points[0].String()); err != nil { - t.Fatalf("Failed to parse point: %v", err) - } -} - -func TestPoint_FieldIterator_Delete_Nothing(t *testing.T) { - points, err := models.ParsePointsString(`m a=1,b=2,c=3`) - if err != nil || len(points) != 1 { - t.Fatal("failed parsing point") - } - - fi := points[0].FieldIterator() - fi.Delete() - - fi.Reset() - - got := toFields(fi) - exp := models.Fields{"a": float64(1), "b": float64(2), "c": float64(3)} - - if !reflect.DeepEqual(got, exp) { - t.Fatalf("Delete failed, got %#v, exp %#v", got, exp) - } - - if _, err = models.ParsePointsString(points[0].String()); err != nil { - t.Fatalf("Failed to parse point: %v", err) - } -} - -func TestPoint_FieldIterator_Delete_Twice(t *testing.T) { - points, err := models.ParsePointsString(`m a=1,b=2,c=3`) - if err != nil || len(points) != 1 { - t.Fatal("failed parsing point") - } - - fi := points[0].FieldIterator() - fi.Next() // a - fi.Next() // b - fi.Delete() - fi.Delete() // no-op - - fi.Reset() - - got := toFields(fi) - exp := models.Fields{"a": float64(1), "c": float64(3)} - - if !reflect.DeepEqual(got, exp) { - t.Fatalf("Delete failed, got %#v, exp %#v", got, exp) - } - - if _, err = models.ParsePointsString(points[0].String()); err != nil { - t.Fatalf("Failed to parse point: %v", err) - } -} - func TestEscapeStringField(t *testing.T) { cases := []struct { in string diff --git a/tsdb/engine/tsm1/engine.go b/tsdb/engine/tsm1/engine.go index 0ab99fc9b7..a53cba6c33 100644 --- a/tsdb/engine/tsm1/engine.go +++ b/tsdb/engine/tsm1/engine.go @@ -37,8 +37,12 @@ func init() { tsdb.RegisterEngine("tsm1", NewEngine) } -// Ensure Engine implements the interface. -var _ tsdb.Engine = &Engine{} +var ( + // Ensure Engine implements the interface. + _ tsdb.Engine = &Engine{} + // Static objects to prevent small allocs. + timeBytes = []byte("time") +) const ( // keyFieldSeparator separates the series key from the field name in the composite key @@ -712,6 +716,11 @@ func (e *Engine) WritePoints(points []models.Point) error { iter := p.FieldIterator() t := p.Time().UnixNano() for iter.Next() { + // Skip fields name "time", they are illegal + if bytes.Equal(iter.FieldKey(), timeBytes) { + continue + } + keyBuf = append(keyBuf[:baseLen], iter.FieldKey()...) var v Value switch iter.Type() { diff --git a/tsdb/shard.go b/tsdb/shard.go index be54e31d76..e351d68246 100644 --- a/tsdb/shard.go +++ b/tsdb/shard.go @@ -547,17 +547,21 @@ func (s *Shard) validateSeriesAndFields(points []models.Point) ([]models.Point, names := make([][]byte, len(points)) tagsSlice := make([]models.Tags, len(points)) + // Drop any series w/ a "time" tag, these are illegal + var j int for i, p := range points { - keys[i] = p.Key() - names[i] = []byte(p.Name()) tags := p.Tags() if v := tags.Get(timeBytes); v != nil { - s.logger.Info(fmt.Sprintf("dropping tag 'time' from '%s'\n", p.PrecisionString(""))) - tags.Delete(timeBytes) - p.SetTags(tags) + dropped++ + continue } - tagsSlice[i] = tags + keys[j] = p.Key() + names[j] = []byte(p.Name()) + tagsSlice[j] = tags + points[j] = points[i] + j++ } + points, keys, names, tagsSlice = points[:j], keys[:j], names[:j], tagsSlice[:j] // Add new series. Check for partial writes. var droppedKeys map[string]struct{} @@ -581,14 +585,14 @@ func (s *Shard) validateSeriesAndFields(points []models.Point) ([]models.Point, iter := p.FieldIterator() for iter.Next() { if bytes.Equal(iter.FieldKey(), timeBytes) { - s.logger.Info(fmt.Sprintf("dropping field 'time' from '%s'\n", p.PrecisionString(""))) - iter.Delete() continue } validField = true + break } if !validField { + dropped++ continue } @@ -604,10 +608,14 @@ func (s *Shard) validateSeriesAndFields(points []models.Point) ([]models.Point, // see if the field definitions need to be saved to the shard mf := s.engine.MeasurementFields(p.Name()) - if mf == nil { var createType influxql.DataType for iter.Next() { + // Skip fields name "time", they are illegal + if bytes.Equal(iter.FieldKey(), timeBytes) { + continue + } + switch iter.Type() { case models.Float: createType = influxql.Float @@ -643,6 +651,12 @@ func (s *Shard) validateSeriesAndFields(points []models.Point) ([]models.Point, // validate field types and encode data for iter.Next() { + + // Skip fields name "time", they are illegal + if bytes.Equal(iter.FieldKey(), timeBytes) { + continue + } + var fieldType influxql.DataType switch iter.Type() { case models.Float: @@ -698,6 +712,10 @@ func (s *Shard) MeasurementFields(name []byte) *MeasurementFields { return s.engine.MeasurementFields(string(name)) } +func (s *Shard) MeasurementExists(name []byte) (bool, error) { + return s.engine.MeasurementExists(name) +} + // WriteTo writes the shard's data to w. func (s *Shard) WriteTo(w io.Writer) (int64, error) { if err := s.ready(); err != nil { diff --git a/tsdb/shard_test.go b/tsdb/shard_test.go index 475fc6d289..833248f5d6 100644 --- a/tsdb/shard_test.go +++ b/tsdb/shard_test.go @@ -1,7 +1,6 @@ package tsdb_test import ( - "bytes" "fmt" "io/ioutil" "os" @@ -21,7 +20,6 @@ import ( _ "github.com/influxdata/influxdb/tsdb/engine" _ "github.com/influxdata/influxdb/tsdb/index" "github.com/influxdata/influxdb/tsdb/index/inmem" - "github.com/uber-go/zap" ) func TestShardWriteAndIndex(t *testing.T) { @@ -220,12 +218,8 @@ func TestWriteTimeTag(t *testing.T) { time.Unix(1, 2), ) - buf := bytes.NewBuffer(nil) - sh.WithLogger(zap.New(zap.NewTextEncoder(), zap.Output(zap.AddSync(buf)))) - if err := sh.WritePoints([]models.Point{pt}); err != nil { - t.Fatalf("unexpected error: %v", err) - } else if got, exp := buf.String(), "dropping field 'time'"; !strings.Contains(got, exp) { - t.Fatalf("unexpected log message: %s", strings.TrimSpace(got)) + if err := sh.WritePoints([]models.Point{pt}); err == nil { + t.Fatal("expected error: got nil") } pt = models.MustNewPoint( @@ -235,12 +229,8 @@ func TestWriteTimeTag(t *testing.T) { time.Unix(1, 2), ) - buf = bytes.NewBuffer(nil) - sh.WithLogger(zap.New(zap.NewTextEncoder(), zap.Output(zap.AddSync(buf)))) if err := sh.WritePoints([]models.Point{pt}); err != nil { t.Fatalf("unexpected error: %v", err) - } else if got, exp := buf.String(), "dropping field 'time'"; !strings.Contains(got, exp) { - t.Fatalf("unexpected log message: %s", strings.TrimSpace(got)) } mf := sh.MeasurementFields([]byte("cpu")) @@ -276,12 +266,13 @@ func TestWriteTimeField(t *testing.T) { time.Unix(1, 2), ) - buf := bytes.NewBuffer(nil) - sh.WithLogger(zap.New(zap.NewTextEncoder(), zap.Output(zap.AddSync(buf)))) - if err := sh.WritePoints([]models.Point{pt}); err != nil { - t.Fatalf("unexpected error: %v", err) - } else if got, exp := buf.String(), "dropping tag 'time'"; !strings.Contains(got, exp) { - t.Fatalf("unexpected log message: %s", strings.TrimSpace(got)) + if err := sh.WritePoints([]models.Point{pt}); err == nil { + t.Fatal("expected error: got nil") + } + + key := models.MakeKey([]byte("cpu"), nil) + if ok, err := sh.MeasurementExists(key); ok && err == nil { + t.Fatal("unexpected series") } }