Report a misspelled bucket parameter instead of deferring it

Follow-up to the review. Accepting bare argument names means anything now
parses, so an unrecognised name reaches the builder as a leftover argument and
was being declined as a syntax check -- which routes it to the legacy engine.
A typo would quietly become a legacy-engine query instead of an error, the
failure mode the earlier review comment was about.

Only the parameters the legacy engine actually implements -- alias, format,
time_zone, min_doc_count, order -- defer now. Anything else is a semantic
check, so the caller sees the message.

Also in this commit: `ifnull` is built from BuiltinFunctionName like the other
constant function names in this file rather than a string literal; the new
capability constant no longer sits between LEGACY_METHOD_QUERY and its javadoc,
which left that constant undocumented.

Correcting the previous commit message: it said the grouping limitation was
specific to two grouping keys and that a single key was fine. That is wrong.
A span over a bare table scan cannot resolve its field either way --

  SELECT date_histogram('field'=ts, 'interval'='1h') AS b, COUNT(*)
  FROM idx GROUP BY date_histogram('field'=ts, 'interval'='1h')

fails on both routes, with or without the select alias, so the bucket always
has to be projected in a derived table first. What the grammar change did fix
is the bare-name spelling, which is what let the two capability gates go. A
test now pins the rejection, asserting only that it is rejected, since the two
routes word the error differently.

Added coverage for the 1M and 1y calendar units Dashboards emits at the wider
zoom levels, which nothing exercised before.

Verified: 13 integration tests, none failing or skipped, on the default route;
`:sql:build` green including the coverage gate.

Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

Author: RyanL1997

integ-test/src/test/java/org/opensearch/sql/sql/DateHistogramBucketFunctionIT.java

@@ -5,6 +5,7 @@
 
 package org.opensearch.sql.sql;
 
+import static org.junit.Assert.assertThrows;
 import static org.opensearch.sql.util.Capability.LEGACY_ENGINE_FALLBACK;
 import static org.opensearch.sql.util.MatcherUtils.rows;
 import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -13,6 +14,7 @@
 import java.io.IOException;
 import org.json.JSONObject;
 import org.junit.Test;
+import org.opensearch.client.ResponseException;
 import org.opensearch.sql.legacy.SQLIntegTestCase;
 import org.opensearch.sql.util.RequiresCapability;
 
@@ -38,7 +40,10 @@ protected void init() throws Exception {
     loadIndex(Index.DATE_HISTOGRAM_TEST);
   }
 
-  /** The planner rejects {@code GROUP BY <expression>}, so the bucket is aliased in a subquery. */
+  /**
+   * The bucket has to be projected in a derived table before it can be grouped on; see {@link
+   * #groupingOnTheBucketWithoutADerivedTableIsRejected}. This is also the shape Dashboards emits.
+   */
   private static String bucketed(String bucketExpr) {
     return "SELECT b, COUNT(*) FROM (SELECT "
         + bucketExpr
@@ -99,10 +104,7 @@ public void intervalSynonymsProduceTheSameBuckets() throws IOException {
     }
   }
 
-  /**
-   * The scan sits in its own derived table because the V2 engine cannot resolve the span's field
-   * otherwise when a second grouping key is present.
-   */
+  /** A second grouping key needs the scan in a derived table of its own as well. */
   @Test
   public void bucketsCombineWithAnAdditionalGroupingKey() throws IOException {
     JSONObject response =
@@ -215,4 +217,31 @@ public void unquotedArgumentNamesReturnNumericBuckets() throws IOException {
 
     verifyDataRowsInOrder(response, rows(0, 19), rows(20, 20), rows(40, 20), rows(60, 13));
   }
+
+  /**
+   * A span over a bare table scan cannot resolve its field, with or without a select alias, so the
+   * bucket always has to be projected in a derived table first. Both routes reject this; only the
+   * message differs, so the assertion is on the rejection alone.
+   */
+  @Test
+  public void groupingOnTheBucketWithoutADerivedTableIsRejected() {
+    assertThrows(
+        ResponseException.class,
+        () ->
+            executeQuery(
+                "SELECT date_histogram('field'=ts, 'interval'='1h') AS b, COUNT(*) FROM "
+                    + IDX
+                    + " GROUP BY date_histogram('field'=ts, 'interval'='1h')"));
+  }
+
+  /** Calendar units: Dashboards emits 1M and 1y at the wider zoom levels. */
+  @Test
+  public void calendarIntervalsBucketByMonthAndYear() throws IOException {
+    verifyDataRows(
+        executeQuery(bucketed("date_histogram('field'=ts, 'interval'='1M')")),
+        rows("2026-01-01 00:00:00", 72));
+    verifyDataRows(
+        executeQuery(bucketed("date_histogram('field'=ts, 'interval'='1y')")),
+        rows("2026-01-01 00:00:00", 72));
+  }
 }

integ-test/src/test/java/org/opensearch/sql/util/Capability.java

@@ -537,21 +537,21 @@ public enum Capability {
   PREPARED_STATEMENT(
       "Prepared statements are unsupported on the analytics-engine route (Calcite path)."),
 
-  /**
-   * FRONTEND: legacy method-query syntax (regexp_query/wildcard_query) is not in the Calcite
-   * grammar.
-   */
   /**
    * FRONTEND: the legacy V1 engine answers call shapes the V2 grammar declines, but only on the
    * default route. Requests reach it when RestSQLQueryAction catches a SyntaxCheckException; the
    * analytics-engine route enters through RestUnifiedQueryAction, which has no such fallback.
    */
   LEGACY_ENGINE_FALLBACK(
-      "A call shape only the legacy V1 engine understands (e.g. positional"
-          + " date_histogram(field=<col>, ...), or an `alias` parameter) can't be answered on the"
-          + " analytics-engine route: reaching that engine depends on RestSQLQueryAction's"
-          + " SyntaxCheckException fallback, and the analytics route does not go through it."),
+      "A call shape only the legacy V1 engine understands (e.g. a date_histogram `alias`"
+          + " parameter) can't be answered on the analytics-engine route: reaching that engine"
+          + " depends on RestSQLQueryAction's SyntaxCheckException fallback, and the analytics"
+          + " route does not go through it."),
 
+  /**
+   * FRONTEND: legacy method-query syntax (regexp_query/wildcard_query) is not in the Calcite
+   * grammar.
+   */
   LEGACY_METHOD_QUERY(
       "Legacy method-query syntax (regexp_query/wildcard_query/query/matchquery) is not in the"
           + " Calcite grammar used by the analytics-engine route."),

sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java

@@ -8,6 +8,7 @@
 import static org.opensearch.sql.ast.dsl.AstDSL.between;
 import static org.opensearch.sql.ast.dsl.AstDSL.not;
 import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName;
+import static org.opensearch.sql.expression.function.BuiltinFunctionName.IFNULL;
 import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL;
 import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL;
 import static org.opensearch.sql.expression.function.BuiltinFunctionName.LIKE;
@@ -81,6 +82,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.antlr.v4.runtime.RuleContext;
@@ -110,6 +112,10 @@
 /** Expression builder to parse text to expression in AST. */
 public class AstExpressionBuilder extends OpenSearchSQLParserBaseVisitor<UnresolvedExpression> {
 
+  /** Bucket parameters a span cannot express, which the legacy engine answers instead. */
+  private static final Set<String> LEGACY_ONLY_BUCKET_ARGS =
+      Set.of("alias", "format", "time_zone", "min_doc_count", "order");
+
   private final AstBuildGuard guard;
 
   public AstExpressionBuilder() {
@@ -173,9 +179,9 @@ public UnresolvedExpression visitScalarFunctionCall(ScalarFunctionCallContext ct
 
   /**
    * Lowers {@code histogram} and {@code date_histogram} to a {@link Span} over the bucketed field.
-   * The grammar admits only the {@code 'name'=value} form, so the positional spelling the legacy
-   * engine has always answered never reaches here -- it stays an unknown scalar function, and
-   * RestSQLQueryAction hands it back to that engine.
+   * Parameters the span cannot express are implemented by the legacy engine through the native
+   * date_histogram aggregation, so those calls are declined as a syntax check to let
+   * RestSQLQueryAction hand them back to it.
    */
   @Override
   public UnresolvedExpression visitBucketFunctionCall(BucketFunctionCallContext ctx) {
@@ -193,13 +199,12 @@ public UnresolvedExpression visitBucketFunctionCall(BucketFunctionCallContext ct
     UnresolvedExpression missing = args.remove("missing");
     Literal interval = intervalOf(args, functionName);
 
-    // Anything left is a parameter this lowering has no equivalent for. The legacy engine
-    // implements all of them -- alias, format, time_zone, min_doc_count, order -- through the
-    // native date_histogram aggregation, so decline in the one way RestSQLQueryAction falls back
-    // on rather than failing a query it can answer.
     if (!args.isEmpty()) {
-      throw new SyntaxCheckException(
-          functionName + " does not accept parameter: " + String.join(", ", args.keySet()));
+      String names = String.join(", ", args.keySet());
+      if (LEGACY_ONLY_BUCKET_ARGS.containsAll(args.keySet())) {
+        throw new SyntaxCheckException(functionName + " does not accept parameter: " + names);
+      }
+      throw new SemanticCheckException(functionName + " does not accept parameter: " + names);
     }
 
     return AstDSL.spanFromSpanLengthLiteral(
@@ -262,7 +267,9 @@ private static UnresolvedExpression normalizeField(UnresolvedExpression field) {
    */
   private static UnresolvedExpression substituteMissing(
       UnresolvedExpression field, UnresolvedExpression missing) {
-    return missing == null ? field : new Function("ifnull", List.of(field, missing));
+    return missing == null
+        ? field
+        : new Function(IFNULL.getName().getFunctionName(), List.of(field, missing));
   }
 
   @Override

sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java

@@ -959,6 +959,20 @@ public void badBucketArgumentIsReportedRatherThanDeferred() {
         () -> buildExprAst("date_histogram('field'=ts, 'interval'='1d', 'interval'='2d')"));
   }
 
+  /**
+   * Bare argument names let anything parse, so a name the legacy engine does not implement either
+   * is a typo. Reporting it keeps a misspelling from silently becoming a legacy-engine query.
+   */
+  @Test
+  public void unknownBucketParameterIsReportedRatherThanDeferred() {
+    assertThrows(
+        SemanticCheckException.class,
+        () -> buildExprAst("date_histogram('field'=ts, 'interval'='1d', 'feild'='ts')"));
+    assertThrows(
+        SemanticCheckException.class,
+        () -> buildExprAst("date_histogram('field'=ts, 'interval'='1d', 'alias'='d', 'nope'=1)"));
+  }
+
   private Node buildExprAst(String expr) {
     return buildExprAst(expr, astExprBuilder);
   }