From a2cec62fa22bf548fdf6c0756f414e037b0caf54 Mon Sep 17 00:00:00 2001
From: Andrej Ocenas <mr.ocenas@gmail.com>
Date: Tue, 15 Mar 2022 17:00:29 +0100
Subject: [PATCH] Prometheus: Fix parsing of binary operations (#46549)

---
 .../prometheus/querybuilder/parsing.test.ts   | 61 +++++++++++++++++++
 .../prometheus/querybuilder/parsing.ts        | 35 +++--------
 2 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts
index 1b86c3884a7..b4dc7919841 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts
+++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts
@@ -444,6 +444,67 @@ describe('buildVisualQueryFromString', () => {
       },
     });
   });
+
+  it('handles multiple binary operations', () => {
+    expect(buildVisualQueryFromString('foo{x="yy"} * metric{y="zz",a="bb"} * metric2')).toEqual({
+      errors: [],
+      query: {
+        metric: 'foo',
+        labels: [{ label: 'x', op: '=', value: 'yy' }],
+        operations: [],
+        binaryQueries: [
+          {
+            operator: '*',
+            query: {
+              metric: 'metric',
+              labels: [
+                { label: 'y', op: '=', value: 'zz' },
+                { label: 'a', op: '=', value: 'bb' },
+              ],
+              operations: [],
+            },
+          },
+          {
+            operator: '*',
+            query: {
+              metric: 'metric2',
+              labels: [],
+              operations: [],
+            },
+          },
+        ],
+      },
+    });
+  });
+
+  it('handles multiple binary operations and scalar', () => {
+    expect(buildVisualQueryFromString('foo{x="yy"} * metric{y="zz",a="bb"} * 2')).toEqual({
+      errors: [],
+      query: {
+        metric: 'foo',
+        labels: [{ label: 'x', op: '=', value: 'yy' }],
+        operations: [
+          {
+            id: '__multiply_by',
+            params: [2],
+          },
+        ],
+        binaryQueries: [
+          {
+            operator: '*',
+            query: {
+              metric: 'metric',
+              labels: [
+                { label: 'y', op: '=', value: 'zz' },
+                { label: 'a', op: '=', value: 'bb' },
+              ],
+              operations: [],
+            },
+          },
+        ],
+      },
+    });
+  });
 });
 
 function noErrors(query: PromVisualQuery) {
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts
index cba3369e44c..c6cb0488b42 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts
+++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts
@@ -350,44 +350,31 @@ function handleBinary(expr: string, node: SyntaxNode, context: Context) {
   const leftNumber = left.getChild('NumberLiteral');
   const rightNumber = right.getChild('NumberLiteral');
 
-  if (leftNumber || rightNumber) {
-    // Scalar case, just add operation.
-    if (leftNumber) {
-      // TODO: this should be already handled in case parent is binary expression as it has to be added to parent
-      //  if query starts with a number that isn't handled now.
-    } else {
-      handleExpression(expr, left, context);
-    }
-
-    if (rightNumber) {
-      // TODO: this should be already handled in case parent is binary expression as it has to be added to parent
-      //  if query starts with a number that isn't handled now.
-      visQuery.operations.push(makeBinOp(opDef, expr, right, binModifier));
-    } else {
-      handleExpression(expr, right, context);
-    }
-    return;
-  }
-
-  const leftBinary = left.getChild('BinaryExpr');
   const rightBinary = right.getChild('BinaryExpr');
 
-  if (leftBinary || rightBinary) {
-    // One of the sides is binary which means we don't really know if there is a query or just chained scalars. So
+  if (leftNumber) {
+    // TODO: this should be already handled in case parent is binary expression as it has to be added to parent
+    //  if query starts with a number that isn't handled now.
+  } else {
+    // If this is binary we don't really know if there is a query or just chained scalars. So
     // we have to traverse a bit deeper to know
     handleExpression(expr, left, context);
+  }
 
+  if (rightNumber) {
+    visQuery.operations.push(makeBinOp(opDef, expr, right, binModifier));
+  } else if (rightBinary) {
     // Due to the way binary ops are parsed we can get a binary operation on the right that starts with a number which
     // is a factor for a current binary operation. So we have to add it as an operation now.
     const leftMostChild = getLeftMostChild(right);
     if (leftMostChild?.name === 'NumberLiteral') {
       visQuery.operations.push(makeBinOp(opDef, expr, leftMostChild, binModifier));
     }
+
     // If we added the first number literal as operation here we still can continue and handle the rest as the first
     // number will be just skipped.
     handleExpression(expr, right, context);
   } else {
-    // Two queries case so we create a binary query.
     visQuery.binaryQueries = visQuery.binaryQueries || [];
     const binQuery = {
       operator: op,
@@ -398,8 +385,6 @@ function handleBinary(expr: string, node: SyntaxNode, context: Context) {
       },
     };
     visQuery.binaryQueries.push(binQuery);
-    // One query is the main query, second is wrapped in the binaryQuery wrapper.
-    handleExpression(expr, left, context);
     handleExpression(expr, right, {
       query: binQuery.query,
       errors: context.errors,