MQE: fix incorrect query results or "found duplicate series for the m…

…atch group" errors when binary operation has unsorted labels in `on` (#9482)

* MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on`

* Add changelog entry

* Address PR feedback: explain purpose of tests

(cherry picked from commit e8e1e13)

CHANGELOG.md

@@ -14,7 +14,7 @@
   * `cortex_alertmanager_alerts`
   * `cortex_alertmanager_silences`
 * [CHANGE] Cache: Deprecate experimental support for Redis as a cache backend. #9453
-* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #9367 #9368 #9398 #9399 #9403 #9417 #9418 #9419 #9420
+* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #9367 #9368 #9398 #9399 #9403 #9417 #9418 #9419 #9420 #9482
 * [FEATURE] Query-frontend: added experimental configuration options `query-frontend.cache-errors` and `query-frontend.results-cache-ttl-for-errors` to allow non-transient responses to be cached. When set to `true` error responses from hitting limits or bad data are cached for a short TTL. #9028
 * [FEATURE] gRPC: Support S2 compression. #9322
   * `-alertmanager.alertmanager-client.grpc-compression=s2`

pkg/streamingpromql/operators/vector_vector_binary_operation.go

@@ -402,6 +402,8 @@ func (b *VectorVectorBinaryOperation) groupKeyFunc() func(labels.Labels) []byte
 	buf := make([]byte, 0, 1024)
 
 	if b.VectorMatching.On {
+		slices.Sort(b.VectorMatching.MatchingLabels)
+
 		return func(l labels.Labels) []byte {
 			return l.BytesWithLabels(buf, b.VectorMatching.MatchingLabels...)
 		}

pkg/streamingpromql/testdata/ours/binary_operators.test

@@ -98,11 +98,27 @@ eval range from 0 to 24m step 6m left_side - on(env, pod) right_side
   {env="test", pod="a"} -9 -18 -27
   {env="test", pod="b"} -36 -45 -54
 
+# Test the same thing again with the grouping labels in a different order.
+# (The implementation of binary operations relies on grouping labels being sorted in some places,
+# so this test exists to ensure this is done correctly.)
+eval range from 0 to 24m step 6m left_side - on(pod, env) right_side
+  {env="prod", pod="a"} -63 -72 -81
+  {env="test", pod="a"} -9 -18 -27
+  {env="test", pod="b"} -36 -45 -54
+
 eval range from 0 to 24m step 6m left_side - ignoring(env, pod) right_side
   {group="baz"} -33 -42 -51
   {group="bar"} -6 -15 -24
   {group="foo"} -69 -78 -87
 
+# Test the same thing again with the grouping labels in a different order.
+# (The implementation of binary operations relies on grouping labels being sorted in some places,
+# so this test exists to ensure this is done correctly.)
+eval range from 0 to 24m step 6m left_side - ignoring(pod, env) right_side
+  {group="baz"} -33 -42 -51
+  {group="bar"} -6 -15 -24
+  {group="foo"} -69 -78 -87
+
 clear
 
 # One-to-one matching, but different series match at different time steps, or not at all