Consolidate grammar/ast tests

Remove GrammarTests in favor of PromqlAstTests to test not just the
grammar but also the semantic validation of the queries, especially
as the latter includes the former.

The invalid tests are currently passing, but there are still some valid
tests that are failing, likely due to incorrect AST assembly.

Author: costin

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/Arithmetics.java

@@ -148,7 +148,7 @@ public static Number mod(Number l, Number r) {
         return l.intValue() % r.intValue();
     }
 
-    static Number negate(Number n) {
+    public static Number negate(Number n) {
         if (n == null) {
             return null;
         }

x-pack/plugin/esql/qa/testFixtures/src/main/resources/promql/grammar/queries-invalid.promql

@@ -145,9 +145,9 @@ foo{1};
 |vector selector must contain at least one non-empty matcher;
 {x=""};
 |vector selector must contain at least one non-empty matcher;
-{x=".*"};
+{x=~".*"};
 |vector selector must contain at least one non-empty matcher;
-{x!".+"};
+{x!~".+"};
 |vector selector must contain at least one non-empty matcher;
 {x!="a"};
 |vector selector must contain at least one non-empty matcher;

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

@@ -14,6 +14,7 @@
 import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedTimestamp;
+import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.Arithmetics;
 import org.elasticsearch.xpack.esql.core.tree.Node;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -261,9 +262,36 @@ public LogicalPlan visitArithmeticUnary(PromqlBaseParser.ArithmeticUnaryContext
         Source source = source(ctx);
         LogicalPlan unary = wrapLiteral(ctx.expression());
 
+        // unary operators do not make sense outside numeric data
+        if (unary instanceof LiteralSelector literalSelector) {
+            Literal literal = literalSelector.literal();
+            Object value = literal.value();
+            DataType dataType = literal.dataType();
+            if (dataType.isNumeric() == false || value instanceof Number == false) {
+                throw new ParsingException(
+                    source,
+                    "Unary expression only allowed on expressions of type numeric or instant vector, got [{}]",
+                    dataType.typeName()
+                );
+            }
+            // optimize negation in case of literals
+            if (ctx.operator.getType() == MINUS) {
+                Number negatedValue = Arithmetics.negate((Number) value);
+                unary = new LiteralSelector(source, new Literal(unary.source(), negatedValue, dataType));
+            }
+        }
+        // forbid range selectors
+        else if (unary instanceof InstantSelector == false) {
+            throw new ParsingException(
+                source,
+                "Unary expression only allowed on expressions of type numeric or instant vector, got [{}]",
+                unary.nodeName()
+            );
+        }
+
+        // For non-literals (vectors), rewrite as 0 - expression
         if (ctx.operator.getType() == MINUS) {
-            // TODO: optimize negation of literal
-            LiteralSelector zero = new LiteralSelector(source, Literal.fromDouble(source, 0.0));
+            LiteralSelector zero = new LiteralSelector(source, Literal.integer(source, 0));
             return new VectorBinaryArithmetic(source, zero, unary, VectorMatch.NONE, VectorBinaryArithmetic.ArithmeticOp.SUB);
         }
 
@@ -311,7 +339,7 @@ public LogicalPlan visitArithmeticBinary(PromqlBaseParser.ArithmeticBinaryContex
         PromqlBaseParser.ModifierContext modifierCtx = ctx.modifier();
         if (modifierCtx != null) {
             // modifiers work only on vectors
-            if (le instanceof RangeSelector || re instanceof RangeSelector) {
+            if (le instanceof InstantSelector == false || re instanceof InstantSelector == false) {
                 throw new ParsingException(source, "Vector matching allowed only between instant vectors");
             }
 

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

@@ -7,25 +7,39 @@
 
 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;
 import org.elasticsearch.xpack.esql.parser.PromqlParser;
 
+import java.io.BufferedReader;
+import java.net.URL;
+import java.util.ArrayList;
 import java.util.List;
 
 import static java.util.Arrays.asList;
 import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.not;
 
 /**
  * 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")
 public class PromqlAstTests extends ESTestCase {
 
+    private static final Logger log = LogManager.getLogger(PromqlAstTests.class);
+
+    // @AwaitsFix(bugUrl = "going through them, one at a time")
     public void testValidQueries() throws Exception {
-        List<Tuple<String, Integer>> lines = PromqlGrammarTests.readQueries("/promql/grammar/queries-valid.promql");
+        List<Tuple<String, Integer>> lines = readQueries("/promql/grammar/queries-valid.promql");
         for (Tuple<String, Integer> line : lines) {
             String q = line.v1();
             try {
@@ -39,37 +53,53 @@ public void testValidQueries() throws Exception {
         }
     }
 
-    public void testQuery() throws Exception {
-        String query = "rate(metric[5m])";
-        new PromqlParser().createStatement(query);
-    }
-
-    @AwaitsFix(bugUrl = "placeholder for individual queries")
-    public void testSingleQuery() throws Exception {
-        String query = "{x=\".*\"}";
-        new PromqlParser().createStatement(query);
-    }
-
-    // @AwaitsFix(bugUrl = "requires parsing validation, not the focus for now")
     public void testUnsupportedQueries() throws Exception {
-        List<Tuple<String, Integer>> lines = PromqlGrammarTests.readQueries("/promql/grammar/queries-invalid.promql");
+        List<Tuple<String, Integer>> lines = readQueries("/promql/grammar/queries-invalid.promql");
         for (Tuple<String, Integer> line : lines) {
             String q = line.v1();
             try {
-                System.out.println("Testing invalid query: " + q);
+                log.trace("Testing invalid query {}", q);
                 PromqlParser parser = new PromqlParser();
                 Exception pe = expectThrowsAnyOf(
                     asList(QlClientException.class, UnsupportedOperationException.class),
                     () -> parser.createStatement(q)
                 );
                 parser.createStatement(q);
-                // System.out.printf(pe.getMessage());
+                log.trace("{}", pe.getMessage());
             } catch (QlClientException | UnsupportedOperationException ex) {
                 // Expected
             }
-            // } catch (AssertionError ae) {
-            // fail(format(null, "Unexpected exception for line {}: [{}] \n {}", line.v2(), line.v1(), ae.getCause()));
-            // }
         }
     }
+
+    static List<Tuple<String, Integer>> readQueries(String source) throws Exception {
+        var urls = EsqlTestUtils.classpathResources(source);
+        assertThat(urls, not(empty()));
+        List<Tuple<String, Integer>> queries = new ArrayList<>();
+
+        StringBuilder query = new StringBuilder();
+        for (URL url : urls) {
+            try (BufferedReader reader = EsqlTestUtils.reader(url)) {
+                String line;
+                int lineNumber = 1;
+
+                while ((line = reader.readLine()) != null) {
+                    // ignore comments
+                    if (line.isEmpty() == false && line.startsWith("//") == false) {
+                        query.append(line);
+
+                        if (line.endsWith(";")) {
+                            query.setLength(query.length() - 1);
+                            queries.add(new Tuple<>(query.toString(), lineNumber));
+                            query.setLength(0);
+                        } else {
+                            query.append("\n");
+                        }
+                    }
+                    lineNumber++;
+                }
+            }
+        }
+        return queries;
+    }
 }