diff --git a/promql/engine.go b/promql/engine.go index 2051774af2..90fc78e8a0 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -720,6 +720,9 @@ func (ev *evaluator) rangeEval(f func([]Value, *EvalNodeHelper) Vector, exprs .. // Make the function call. enh.ts = ts result := f(args, enh) + if result.ContainsSameLabelset() { + ev.errorf("vector cannot contain metrics with the same labelset") + } enh.out = result[:0] // Reuse result vector. // If this could be an instant query, shortcut so as not to change sort order. if ev.endTimestamp == ev.startTimestamp { @@ -883,6 +886,10 @@ func (ev *evaluator) eval(expr Expr) Value { mat = append(mat, ss) } } + if mat.ContainsSameLabelset() { + ev.errorf("vector cannot contain metrics with the same labelset") + } + putPointSlice(points) return mat @@ -898,6 +905,9 @@ func (ev *evaluator) eval(expr Expr) Value { mat[i].Points[j].V = -mat[i].Points[j].V } } + if mat.ContainsSameLabelset() { + ev.errorf("vector cannot contain metrics with the same labelset") + } } return mat diff --git a/promql/functions.go b/promql/functions.go index 46fa473974..21757da343 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -734,7 +734,6 @@ func funcLabelReplace(vals []Value, args Expressions, enh *EvalNodeHelper) Vecto enh.dmn = make(map[uint64]labels.Labels, len(enh.out)) } - outSet := make(map[uint64]struct{}, len(vector)) for _, el := range vector { h := el.Metric.Hash() var outMetric labels.Labels @@ -759,17 +758,10 @@ func funcLabelReplace(vals []Value, args Expressions, enh *EvalNodeHelper) Vecto } } - outHash := outMetric.Hash() - if _, ok := outSet[outHash]; ok { - panic(fmt.Errorf("duplicated label set in output of label_replace(): %s", el.Metric)) - } else { - enh.out = append(enh.out, - Sample{ - Metric: outMetric, - Point: Point{V: el.Point.V}, - }) - outSet[outHash] = struct{}{} - } + enh.out = append(enh.out, Sample{ + Metric: outMetric, + Point: Point{V: el.Point.V}, + }) } return enh.out } @@ -808,7 +800,6 @@ func funcLabelJoin(vals []Value, args Expressions, enh *EvalNodeHelper) Vector { panic(fmt.Errorf("invalid destination label name in label_join(): %s", dst)) } - outSet := make(map[uint64]struct{}, len(vector)) srcVals := make([]string, len(srcLabels)) for _, el := range vector { h := el.Metric.Hash() @@ -833,17 +824,11 @@ func funcLabelJoin(vals []Value, args Expressions, enh *EvalNodeHelper) Vector { outMetric = lb.Labels() enh.dmn[h] = outMetric } - outHash := outMetric.Hash() - if _, exists := outSet[outHash]; exists { - panic(fmt.Errorf("duplicated label set in output of label_join(): %s", el.Metric)) - } else { - enh.out = append(enh.out, Sample{ - Metric: outMetric, - Point: Point{V: el.Point.V}, - }) - outSet[outHash] = struct{}{} - } + enh.out = append(enh.out, Sample{ + Metric: outMetric, + Point: Point{V: el.Point.V}, + }) } return enh.out } diff --git a/promql/testdata/functions.test b/promql/testdata/functions.test index 70baae5735..2ecad97889 100644 --- a/promql/testdata/functions.test +++ b/promql/testdata/functions.test @@ -514,3 +514,11 @@ eval instant at 0m days_in_month(vector(1454284800)) eval instant at 0m days_in_month(vector(1485907200)) {} 28 +clear + +# Test duplicate labelset in promql output. +load 5m + testmetric1{src="a",dst="b"} 0 + testmetric2{src="a",dst="b"} 1 + +eval_fail instant at 0m changes({__name__=~'testmetric1|testmetric2'}[5m]) \ No newline at end of file diff --git a/promql/testdata/legacy.test b/promql/testdata/legacy.test index daa1e473e5..5ed4421334 100644 --- a/promql/testdata/legacy.test +++ b/promql/testdata/legacy.test @@ -357,3 +357,12 @@ load 1h eval instant at 0h testmetric testmetric{aa="bb"} 1 testmetric{a="abb"} 2 + +clear + +# Test duplicate labelset in promql output. +load 5m + testmetric1{src="a",dst="b"} 0 + testmetric2{src="a",dst="b"} 1 + +eval_fail instant at 0m ceil({__name__=~'testmetric1|testmetric2'}) \ No newline at end of file diff --git a/promql/testdata/operators.test b/promql/testdata/operators.test index 7bac547801..067e86839f 100644 --- a/promql/testdata/operators.test +++ b/promql/testdata/operators.test @@ -367,3 +367,12 @@ eval instant at 5m metricA + ignoring() metricB eval instant at 5m metricA + metricB {baz="meh"} 7 + +clear + +# Test duplicate labelset in promql output. +load 5m + testmetric1{src="a",dst="b"} 0 + testmetric2{src="a",dst="b"} 1 + +eval_fail instant at 0m -{__name__=~'testmetric1|testmetric2'} \ No newline at end of file diff --git a/promql/value.go b/promql/value.go index fe902cd23a..a577030815 100644 --- a/promql/value.go +++ b/promql/value.go @@ -140,6 +140,22 @@ func (vec Vector) String() string { return strings.Join(entries, "\n") } +// ContainsSameLabelset checks if a vector has samples with the same labelset +// Such a behaviour is semantically undefined +// https://github.com/prometheus/prometheus/issues/4562 +func (vec Vector) ContainsSameLabelset() bool { + l := make(map[uint64]struct{}, len(vec)) + for _, s := range vec { + hash := s.Metric.Hash() + if _, ok := l[hash]; ok { + return true + } else { + l[hash] = struct{}{} + } + } + return false +} + // Matrix is a slice of Seriess that implements sort.Interface and // has a String method. type Matrix []Series @@ -159,6 +175,22 @@ func (m Matrix) Len() int { return len(m) } func (m Matrix) Less(i, j int) bool { return labels.Compare(m[i].Metric, m[j].Metric) < 0 } func (m Matrix) Swap(i, j int) { m[i], m[j] = m[j], m[i] } +// ContainsSameLabelset checks if a matrix has samples with the same labelset +// Such a behaviour is semantically undefined +// https://github.com/prometheus/prometheus/issues/4562 +func (m Matrix) ContainsSameLabelset() bool { + l := make(map[uint64]struct{}, len(m)) + for _, ss := range m { + hash := ss.Metric.Hash() + if _, ok := l[hash]; ok { + return true + } else { + l[hash] = struct{}{} + } + } + return false +} + // Result holds the resulting value of an execution or an error // if any occurred. type Result struct {