SQL: Remove exceptions from Analyzer (#38260)

Instead of throwing an exception, use an unresolved attribute to pass
the message to the Verifier.
Additionally improve the parser to save the extended source for the
Aggregate and OrderBy.

Close #38208

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

@@ -5,7 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.analysis.analyzer;
 
-import org.elasticsearch.xpack.sql.analysis.AnalysisException;
+import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.xpack.sql.analysis.analyzer.Verifier.Failure;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
 import org.elasticsearch.xpack.sql.capabilities.Resolvables;
@@ -16,7 +16,7 @@ import org.elasticsearch.xpack.sql.expression.AttributeSet;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.expression.FieldAttribute;
-import org.elasticsearch.xpack.sql.expression.Literal;
+import org.elasticsearch.xpack.sql.expression.Foldables;
 import org.elasticsearch.xpack.sql.expression.NamedExpression;
 import org.elasticsearch.xpack.sql.expression.Order;
 import org.elasticsearch.xpack.sql.expression.SubQueryExpression;
@@ -516,6 +516,7 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
                 int max = ordinalReference.size();
 
                 for (Order order : orderBy.order()) {
+                    Expression child = order.child();
                     Integer ordinal = findOrdinal(order.child());
                     if (ordinal != null) {
                         changed = true;
@@ -524,7 +525,11 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
                                     order.nullsPosition()));
                         }
                         else {
-                            throw new AnalysisException(order, "Invalid %d specified in OrderBy (valid range is [1, %d])", ordinal, max);
+                            // report error
+                            String message = LoggerMessageFormat.format("Invalid ordinal [{}] specified in [{}] (valid range is [1, {}])",
+                                    ordinal, orderBy.sourceText(), max);
+                            UnresolvedAttribute ua = new UnresolvedAttribute(child.source(), orderBy.sourceText(), null, message);
+                            newOrder.add(new Order(order.source(), ua, order.direction(), order.nullsPosition()));
                         }
                     }
                     else {
@@ -551,17 +556,24 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
                     Integer ordinal = findOrdinal(exp);
                     if (ordinal != null) {
                         changed = true;
+                        String errorMessage = null;
                         if (ordinal > 0 && ordinal <= max) {
                             NamedExpression reference = aggregates.get(ordinal - 1);
                             if (containsAggregate(reference)) {
-                                throw new AnalysisException(exp, "Group ordinal " + ordinal + " refers to an aggregate function "
+                                errorMessage = LoggerMessageFormat.format(
-                                        + reference.nodeName() + " which is not compatible/allowed with GROUP BY");
+                                        "Ordinal [{}] in [{}] refers to an invalid argument, aggregate function [{}]",
+                                        ordinal, agg.sourceText(), reference.sourceText());
+
+                            } else {
+                                newGroupings.add(reference);
                             }
-                            newGroupings.add(reference);
                         }
                         else {
-                            throw new AnalysisException(exp, "Invalid ordinal " + ordinal
+                            errorMessage = LoggerMessageFormat.format("Invalid ordinal [{}] specified in [{}] (valid range is [1, {}])",
-                                    + " specified in Aggregate (valid range is [1, " + max + "])");
+                                    ordinal, agg.sourceText(), max);
+                        }
+                        if (errorMessage != null) {
+                            newGroupings.add(new UnresolvedAttribute(exp.source(), agg.sourceText(), null, errorMessage));
                         }
                     }
                     else {
@@ -576,10 +588,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
         }
 
         private Integer findOrdinal(Expression expression) {
-            if (expression instanceof Literal) {
+            if (expression.foldable()) {
-                Literal l = (Literal) expression;
+                if (expression.dataType().isInteger()) {
-                if (l.dataType().isInteger()) {
+                    Object v = Foldables.valueOf(expression);
-                    Object v = l.value();
                     if (v instanceof Number) {
                         return Integer.valueOf(((Number) v).intValue());
                     }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/AbstractBuilder.java

@@ -102,6 +102,15 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
         return new Source(new Location(start.getLine(), start.getCharPositionInLine()), text);
     }
 
+    static Source source(TerminalNode begin, ParserRuleContext end) {
+        Check.notNull(begin, "begin is null");
+        Check.notNull(end, "end is null");
+        Token start = begin.getSymbol();
+        Token stop = end.stop != null ? end.stop : start;
+        String text = start.getInputStream().getText(new Interval(start.getStartIndex(), stop.getStopIndex()));
+        return new Source(new Location(start.getLine(), start.getCharPositionInLine()), text);
+    }
+
     /**
      * Retrieves the raw text of the node (without interpreting it as a string literal).
      */

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.parser;
 
+import org.antlr.v4.runtime.ParserRuleContext;
 import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.tree.TerminalNode;
 import org.elasticsearch.xpack.sql.expression.Expression;
@@ -16,10 +17,12 @@ import org.elasticsearch.xpack.sql.parser.SqlBaseParser.AliasedQueryContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.AliasedRelationContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.FromClauseContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.GroupByContext;
+import org.elasticsearch.xpack.sql.parser.SqlBaseParser.GroupingElementContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.JoinCriteriaContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.JoinRelationContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.LimitClauseContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.NamedQueryContext;
+import org.elasticsearch.xpack.sql.parser.SqlBaseParser.OrderByContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QueryContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QueryNoWithContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QuerySpecificationContext;
@@ -85,7 +88,9 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
         LogicalPlan plan = plan(ctx.queryTerm());
 
         if (!ctx.orderBy().isEmpty()) {
-            plan = new OrderBy(source(ctx.ORDER()), plan, visitList(ctx.orderBy(), Order.class));
+            List<OrderByContext> orders = ctx.orderBy();
+            OrderByContext endContext = orders.get(orders.size() - 1);
+            plan = new OrderBy(source(ctx.ORDER(), endContext), plan, visitList(ctx.orderBy(), Order.class));
         }
 
         LimitClauseContext limitClause = ctx.limitClause();
@@ -131,8 +136,10 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
             if (groupByAll != null) {
                 throw new ParsingException(source(groupByAll), "GROUP BY ALL is not supported");
             }
-            List<Expression> groupBy = expressions(groupByCtx.groupingElement());
+            List<GroupingElementContext> groupingElement = groupByCtx.groupingElement();
-            query = new Aggregate(source(groupByCtx), query, groupBy, selectTarget);
+            List<Expression> groupBy = expressions(groupingElement);
+            ParserRuleContext endSource = groupingElement.isEmpty() ? groupByCtx : groupingElement.get(groupingElement.size() - 1);
+            query = new Aggregate(source(ctx.GROUP(), endSource), query, groupBy, selectTarget);
         }
         else if (!selectTarget.isEmpty()) {
             query = new Project(source(ctx.selectItem(0)), query, selectTarget);

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

@@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.analysis.analyzer;
 
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.TestUtils;
-import org.elasticsearch.xpack.sql.analysis.AnalysisException;
 import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolverTests;
@@ -42,7 +41,7 @@ public class VerifierErrorMessagesTests extends ESTestCase {
 
     private String error(IndexResolution getIndexResult, String sql) {
         Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), getIndexResult, new Verifier(new Metrics()));
-        AnalysisException e = expectThrows(AnalysisException.class, () -> analyzer.analyze(parser.createStatement(sql), true));
+        VerificationException e = expectThrows(VerificationException.class, () -> analyzer.analyze(parser.createStatement(sql), true));
         assertTrue(e.getMessage().startsWith("Found "));
         String header = "Found 1 problem(s)\nline ";
         return e.getMessage().substring(header.length());
@@ -170,6 +169,11 @@ public class VerifierErrorMessagesTests extends ESTestCase {
         assertEquals("1:41: Unknown column [xxx]", error("SELECT * FROM test GROUP BY DAY_OF_YEAR(xxx)"));
     }
 
+    public void testInvalidOrdinalInOrderBy() {
+        assertEquals("1:56: Invalid ordinal [3] specified in [ORDER BY 2, 3] (valid range is [1, 2])",
+                error("SELECT bool, MIN(int) FROM test GROUP BY 1 ORDER BY 2, 3"));
+    }
+
     public void testFilterOnUnknownColumn() {
         assertEquals("1:26: Unknown column [xxx]", error("SELECT * FROM test WHERE xxx = 1"));
     }
@@ -239,6 +243,21 @@ public class VerifierErrorMessagesTests extends ESTestCase {
             error("SELECT MAX(int) FROM test GROUP BY text HAVING MAX(int) > 10 ORDER BY bool"));
     }
 
+    public void testGroupByOrdinalPointingToAggregate() {
+        assertEquals("1:42: Ordinal [2] in [GROUP BY 2] refers to an invalid argument, aggregate function [MIN(int)]",
+                error("SELECT bool, MIN(int) FROM test GROUP BY 2"));
+    }
+
+    public void testGroupByInvalidOrdinal() {
+        assertEquals("1:42: Invalid ordinal [3] specified in [GROUP BY 3] (valid range is [1, 2])",
+                error("SELECT bool, MIN(int) FROM test GROUP BY 3"));
+    }
+
+    public void testGroupByNegativeOrdinal() {
+        assertEquals("1:42: Invalid ordinal [-1] specified in [GROUP BY -1] (valid range is [1, 2])",
+                error("SELECT bool, MIN(int) FROM test GROUP BY -1"));
+    }
+
     public void testGroupByOrderByAliasedInSelectAllowed() {
         LogicalPlan lp = accept("SELECT text t FROM test GROUP BY text ORDER BY t");
         assertNotNull(lp);