From 6fd39e200089bfcb3bca2bcda3cdad6955207e4b Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Fri, 12 May 2023 15:22:27 -0700
Subject: [PATCH] Revert "lib/protoparser: fix skip csv line when metric can be
 collect from the line (#4298)"

This reverts commit 410ae99c2ee43e505aa20b32530045a657ff6a23.

Reason for revert: the commit masks the real issue instead of fixing it.
The real issue is that the scanner.NextColumn() skips the last column if it is empty.

The commit also introduces two bugs:

- a panic if all the metric values in CSV line are empty
- silent import of CSV lines with too small number of columns

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4048
See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4298
---
 docs/CHANGELOG.md                        |  1 -
 lib/protoparser/csvimport/parser.go      |  6 ++--
 lib/protoparser/csvimport/parser_test.go | 36 ------------------------
 3 files changed, 4 insertions(+), 39 deletions(-)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 4e9ca6ec4..c25c57769 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -74,7 +74,6 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): fix compatibility with Windows OS. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/70).
 * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): fix performance issue when migrating data from VictoriaMetrics according to [these docs](https://docs.victoriametrics.com/vmctl.html#migrating-data-from-victoriametrics). Add the ability to speed up the data migration via `--vm-native-disable-retries` command-line flag. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4092).
 * BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html): fix bug with duplicated labels during stream aggregation via single-node VictoriaMetrics. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4277).
-* BUGFIX: [csvimport](https://docs.victoriametrics.com/#how-to-import-csv-data): properly parse [csv line](https://docs.victoriametrics.com/#how-to-import-csv-data) when value in the last column is empty. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4048).
 * BUGFIX: [relabeling](https://docs.victoriametrics.com/relabeling.html): properly validate labels input on Metric Relabel Debug page in [VMUI](https://docs.victoriametrics.com/#vmui). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4284).
 
 ## [v1.90.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.90.0)
diff --git a/lib/protoparser/csvimport/parser.go b/lib/protoparser/csvimport/parser.go
index decdce513..56e7560b5 100644
--- a/lib/protoparser/csvimport/parser.go
+++ b/lib/protoparser/csvimport/parser.go
@@ -118,8 +118,7 @@ func parseRows(sc *scanner, dst []Row, tags []Tag, metrics []metric, cds []Colum
 				Value: value,
 			})
 		}
-
-		if col < uint(len(cds)) && sc.Error == nil && len(metrics) == 0 {
+		if col < uint(len(cds)) && sc.Error == nil {
 			sc.Error = fmt.Errorf("missing columns in the csv line %q; got %d columns; want at least %d columns", line, col, len(cds))
 		}
 		if sc.Error != nil {
@@ -127,6 +126,9 @@ func parseRows(sc *scanner, dst []Row, tags []Tag, metrics []metric, cds []Colum
 			invalidLines.Inc()
 			continue
 		}
+		if len(metrics) == 0 {
+			logger.Panicf("BUG: expecting at least a single metric in columnDescriptors=%#v", cds)
+		}
 		r.Metric = metrics[0].Name
 		r.Tags = tags[tagsLen:]
 		r.Value = metrics[0].Value
diff --git a/lib/protoparser/csvimport/parser_test.go b/lib/protoparser/csvimport/parser_test.go
index 55635348f..342a1d357 100644
--- a/lib/protoparser/csvimport/parser_test.go
+++ b/lib/protoparser/csvimport/parser_test.go
@@ -55,42 +55,6 @@ func TestRowsUnmarshalSuccess(t *testing.T) {
 			t.Fatalf("unexpected rows on the second unmarshal;\ngot\n%v\nwant\n%v", rs.Rows, rowsExpected)
 		}
 	}
-	f("1:label:mytest,2:time:unix_ns,3:metric:metric_1,4:metric:metric_2", "test,1677632461449998000,,", nil)
-	f("1:time:unix_ns,2:metric:metric_1,3:metric:metric_2", "1677632461449998000,,", nil)
-	f("1:time:unix_ns,2:metric:metric_1,3:metric:metric_2", "1677632461449998000,1,", []Row{
-		{
-			Metric:    "metric_1",
-			Value:     1,
-			Timestamp: 1677632461449,
-		},
-	})
-	f("1:time:unix_ns,2:metric:metric_1,3:metric:metric_2",
-		`
-1677632461449998000,1,1
-1677633061449998000,1,
-1677633661449998000,,1`, []Row{
-			{
-				Metric:    "metric_1",
-				Value:     1,
-				Timestamp: 1677632461449,
-			},
-			{
-				Metric:    "metric_2",
-				Value:     1,
-				Timestamp: 1677632461449,
-			},
-			{
-				Metric:    "metric_1",
-				Value:     1,
-				Timestamp: 1677633061449,
-			},
-			{
-				Metric:    "metric_2",
-				Value:     1,
-				Timestamp: 1677633661449,
-			},
-		})
-
 	f("1:metric:foo", "", nil)
 	f("1:metric:foo", `123`, []Row{
 		{