Merge remote-tracking branch 'felixbarny/promql-parameter-parsing' into promql-parameter-parsing

Author: felixbarny

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/promql/predicate/operator/VectorBinaryOperator.java

@@ -14,8 +14,6 @@
 import org.elasticsearch.xpack.esql.core.expression.function.Function;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
-import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper;
-import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
 import org.elasticsearch.xpack.esql.plan.logical.BinaryPlan;
 import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.esql.plan.logical.promql.selector.LabelMatcher;
@@ -163,9 +161,7 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) return false;
         if (super.equals(o)) {
             VectorBinaryOperator that = (VectorBinaryOperator) o;
-            return dropMetricName == that.dropMetricName
-                && Objects.equals(match, that.match)
-                && Objects.equals(binaryOp, that.binaryOp);
+            return dropMetricName == that.dropMetricName && Objects.equals(match, that.match) && Objects.equals(binaryOp, that.binaryOp);
         }
         return false;
     }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/promql/predicate/operator/comparison/VectorBinaryComparison.java

@@ -47,7 +47,14 @@ public ScalarFunctionFactory asFunction() {
     private final ComparisonOp op;
     private final boolean boolMode;
 
-    public VectorBinaryComparison(Source source, LogicalPlan left, LogicalPlan right, VectorMatch match, boolean boolMode, ComparisonOp op) {
+    public VectorBinaryComparison(
+        Source source,
+        LogicalPlan left,
+        LogicalPlan right,
+        VectorMatch match,
+        boolean boolMode,
+        ComparisonOp op
+    ) {
         super(source, left, right, match, boolMode == false, op);
         this.op = op;
         this.boolMode = boolMode;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/promql/PromqlExpressionBuilder.java

@@ -173,10 +173,7 @@ public Duration visitDuration(DurationContext ctx) {
                 }
 
                 // Non-literal LogicalPlan
-                throw new ParsingException(
-                    source(ctx),
-                    "Duration must be a constant expression"
-                );
+                throw new ParsingException(source(ctx), "Duration must be a constant expression");
             }
             case Expression e -> {
                 // Fallback for Expression (shouldn't happen with new logic)

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/promql/PromqlFoldingUtils.java

@@ -64,11 +64,7 @@ private static Duration arithmetics(Source source, Duration left, Duration right
         Duration result = switch (op) {
             case ADD -> left.plus(right);
             case SUB -> left.minus(right);
-            default -> throw new ParsingException(
-                source,
-                "Operation [{}] not supported between two durations",
-                op
-            );
+            default -> throw new ParsingException(source, "Operation [{}] not supported between two durations", op);
         };
 
         return result;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/promql/PromqlLogicalPlanBuilder.java

@@ -548,8 +548,7 @@ public Duration visitSubqueryResolution(PromqlBaseParser.SubqueryResolutionConte
                 return duration;
             }
 
-            throw new ParsingException(source(ctx), "Expected duration result, got [{}]",
-                result.getClass().getSimpleName());
+            throw new ParsingException(source(ctx), "Expected duration result, got [{}]", result.getClass().getSimpleName());
         }
 
         // Just COLON with no resolution - use default

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/promql/selector/Selector.java

@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.esql.plan.logical.promql.PlaceholderRelation;
 
 import java.io.IOException;
-import java.sql.Time;
 import java.util.List;
 import java.util.Objects;
 

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/promql/PromqlLogicalPlanOptimizerTests.java

@@ -13,8 +13,8 @@
 import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
 import org.elasticsearch.xpack.esql.analysis.Analyzer;
 import org.elasticsearch.xpack.esql.analysis.AnalyzerContext;
-import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.expression.predicate.regex.RegexMatch;
+import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
 import org.elasticsearch.xpack.esql.expression.function.scalar.string.StartsWith;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.In;
@@ -280,26 +280,26 @@ public void testGrammar() {
         System.out.println(plan);
     }
 
-//    public void testPromqlArithmetricOperators() {
-//        // TODO doesn't parse
-//        //  line 1:27: Invalid query '1+1'[ArithmeticBinaryContext] given; expected LogicalPlan but found VectorBinaryArithmetic
-//        assertThat(
-//            error("TS test | PROMQL step 5m (1+1)", tsdb),
-//            equalTo("1:1: arithmetic operators are not supported at this time [foo]")
-//        );
-//        assertThat(
-//            error("TS test | PROMQL step 5m ( foo and bar )", tsdb),
-//            equalTo("1:1: arithmetic operators are not supported at this time [foo]")
-//        );
-//        assertThat(
-//            error("TS test | PROMQL step 5m (1+foo)", tsdb),
-//            equalTo("1:1: arithmetic operators are not supported at this time [foo]")
-//        );
-//        assertThat(
-//            error("TS test | PROMQL step 5m (foo+bar)", tsdb),
-//            equalTo("1:1: arithmetic operators are not supported at this time [foo]")
-//        );
-//    }
+    // public void testPromqlArithmetricOperators() {
+    // // TODO doesn't parse
+    // // line 1:27: Invalid query '1+1'[ArithmeticBinaryContext] given; expected LogicalPlan but found VectorBinaryArithmetic
+    // assertThat(
+    // error("TS test | PROMQL step 5m (1+1)", tsdb),
+    // equalTo("1:1: arithmetic operators are not supported at this time [foo]")
+    // );
+    // assertThat(
+    // error("TS test | PROMQL step 5m ( foo and bar )", tsdb),
+    // equalTo("1:1: arithmetic operators are not supported at this time [foo]")
+    // );
+    // assertThat(
+    // error("TS test | PROMQL step 5m (1+foo)", tsdb),
+    // equalTo("1:1: arithmetic operators are not supported at this time [foo]")
+    // );
+    // assertThat(
+    // error("TS test | PROMQL step 5m (foo+bar)", tsdb),
+    // equalTo("1:1: arithmetic operators are not supported at this time [foo]")
+    // );
+    // }
 
     protected LogicalPlan planPromql(String query) {
         var analyzed = tsAnalyzer.analyze(parser.createStatement(query));

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/promql/PromqlAstTests.java

@@ -7,12 +7,10 @@
 
 package org.elasticsearch.xpack.esql.parser.promql;
 
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.xpack.esql.EsqlTestUtils;
 import org.elasticsearch.xpack.esql.core.QlClientException;
 import org.elasticsearch.xpack.esql.parser.ParsingException;
@@ -32,7 +30,7 @@
  * Test for checking the overall grammar by throwing a number of valid queries at the parser to see whether any exception is raised.
  * In time, the queries themselves get to be checked against the actual execution model and eventually against the expected results.
  */
-//@TestLogging(reason = "debug", value = "org.elasticsearch.xpack.esql.parser.promql:TRACE")
+// @TestLogging(reason = "debug", value = "org.elasticsearch.xpack.esql.parser.promql:TRACE")
 public class PromqlAstTests extends ESTestCase {
 
     private static final Logger log = LogManager.getLogger(PromqlAstTests.class);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/promql/PromqlParamsTests.java

@@ -71,10 +71,7 @@ public void testZeroStep() {
     }
 
     public void testNegativeStep() {
-        ParsingException e = assertThrows(
-            ParsingException.class,
-            () -> parse("TS test | PROMQL step `-1` (avg(foo))")
-        );
+        ParsingException e = assertThrows(ParsingException.class, () -> parse("TS test | PROMQL step `-1` (avg(foo))"));
         assertThat(
             e.getMessage(),
             containsString("invalid parameter \"step\": zero or negative query resolution step widths are not accepted")
@@ -113,10 +110,7 @@ public void testUnknownParameter() {
     }
 
     public void testUnknownParameterNoSuggestion() {
-        ParsingException e = assertThrows(
-            ParsingException.class,
-            () -> parse("TS test | PROMQL foo 1 (avg(foo))")
-        );
+        ParsingException e = assertThrows(ParsingException.class, () -> parse("TS test | PROMQL foo 1 (avg(foo))"));
         assertThat(e.getMessage(), containsString("Unknown parameter [foo]"));
     }
 

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/promql/PromqlParserUtilsTests.java

@@ -53,24 +53,14 @@ public void testTimeValueCombined() throws Exception {
         assertEquals(ofDays(365).plus(ofSeconds(6)), parseTimeValue("1y6s"));
         assertEquals(ofDays(365).plus(ofMillis(7)), parseTimeValue("1y7ms"));
 
-        assertEquals(
-            ofDays(365).plus(ofDays(14)).plus(ofDays(3)),
-            parseTimeValue("1y2w3d")
-        );
+        assertEquals(ofDays(365).plus(ofDays(14)).plus(ofDays(3)), parseTimeValue("1y2w3d"));
 
-        assertEquals(
-            ofDays(365).plus(ofDays(3)).plus(ofHours(4)),
-            parseTimeValue("1y3d4h")
-        );
+        assertEquals(ofDays(365).plus(ofDays(3)).plus(ofHours(4)), parseTimeValue("1y3d4h"));
 
-        assertEquals(
-            ofDays(365).plus(ofMinutes(5)).plus(ofSeconds(6)),
-            parseTimeValue("1y5m6s")
-        );
+        assertEquals(ofDays(365).plus(ofMinutes(5)).plus(ofSeconds(6)), parseTimeValue("1y5m6s"));
 
         assertEquals(
-            ofDays(365).plus(ofDays(7)).plus(ofDays(1)).plus(ofHours(1))
-                .plus(ofMinutes(1)).plus(ofSeconds(1)).plus(ofMillis(1)),
+            ofDays(365).plus(ofDays(7)).plus(ofDays(1)).plus(ofHours(1)).plus(ofMinutes(1)).plus(ofSeconds(1)).plus(ofMillis(1)),
             parseTimeValue("1y1w1d1h1m1s1ms")
         );