[BugFix] Fix empty SQL query failing with internal NullPointerException (#5879)

api/src/test/java/org/opensearch/sql/api/parser/UnifiedQueryParserTest.java

@@ -83,6 +83,21 @@ public void testSyntaxErrorThrows() {
     assertThrows(SyntaxCheckException.class, () -> context.getParser().parse("not a valid query"));
   }
 
+  @Test
+  public void sqlQueryWithoutStatementThrowsEmptyQueryError() throws Exception {
+    try (UnifiedQueryContext sql =
+        UnifiedQueryContext.builder()
+            .language(QueryType.SQL)
+            .catalog(DEFAULT_CATALOG, testSchema)
+            .build()) {
+      for (String query : new String[] {"", "   ", "/* only a comment */", "-- line", ";"}) {
+        IllegalArgumentException e =
+            assertThrows(IllegalArgumentException.class, () -> sql.getParser().parse(query));
+        assertEquals("Query is empty", e.getMessage());
+      }
+    }
+  }
+
   @Test
   public void deeplyNestedExpressionShouldBeRejected() throws Exception {
     String key = Settings.Key.MAX_EXPRESSION_DEPTH.getKeyValue();

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

@@ -117,6 +117,26 @@ public void testSqlQueryWithArrayPayloadShouldFailWithBadRequest() throws IOExce
     whenExecuteMalformedPayload();
   }
 
+  // A query with no statement used to parse successfully and fail later with an internal
+  // NullPointerException. It must be rejected as an empty query with a 400, without falling back
+  // to the legacy engine.
+  @Test
+  public void testQueryWithoutStatementShouldFailWithBadRequest() throws IOException {
+    for (String query : new String[] {"", "   ", "/* only a comment */", "-- line", ";"}) {
+      for (boolean explain : new boolean[] {false, true}) {
+        Request request =
+            getSqlRequest(String.format(Locale.ROOT, "{\"query\": \"%s\"}", query), explain);
+        ResponseException e =
+            assertThrows(ResponseException.class, () -> client().performRequest(request));
+        String context = "explain=" + explain + ", query=[" + query + "]";
+        assertEquals(context, 400, e.getResponse().getStatusLine().getStatusCode());
+        assertTrue(context, e.getMessage().contains("IllegalArgumentException"));
+        assertTrue(context, e.getMessage().contains("Query is empty"));
+        assertFalse(context, e.getMessage().contains("NullPointerException"));
+      }
+    }
+  }
+
   private static void whenExecuteMalformedPayload() throws IOException {
     Request request = new Request("POST", QUERY_API_ENDPOINT);
     request.setJsonEntity("{ \"query\": [\"SELECT * FROM opensearch-sql_test_index_account\"] }");

integ-test/src/yamlRestTest/resources/rest-api-spec/api/sql.explain.json

@@ -0,0 +1,22 @@
+{
+  "sql.explain": {
+    "documentation": {
+      "url": "https://github.com/opensearch-project/sql/blob/main/docs/user/index.rst",
+      "description": "OpenSearch SQL Reference Manual"
+    },
+    "stability" : "stable",
+    "url": {
+      "paths": [
+        {
+          "path" : "/_plugins/_sql/_explain",
+          "methods" : ["POST"]
+        }
+      ]
+    },
+    "params": {},
+    "body": {
+      "description": "SQL Explain Query",
+      "required":true
+    }
+  }
+}

integ-test/src/yamlRestTest/resources/rest-api-spec/api/sql.json

@@ -0,0 +1,27 @@
+{
+  "sql": {
+    "documentation": {
+      "url": "https://github.com/opensearch-project/sql/blob/main/docs/user/index.rst",
+      "description": "OpenSearch SQL Reference Manual"
+    },
+    "stability" : "stable",
+    "url": {
+      "paths": [
+        {
+          "path" : "/_plugins/_sql",
+          "methods" : ["POST"]
+        }
+      ]
+    },
+    "params": {
+      "format":{
+        "type":"string",
+        "description":"response format: jdbc, csv, raw"
+      }
+    },
+    "body": {
+      "description": "SQL Query",
+      "required":true
+    }
+  }
+}

integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5879.yml

@@ -0,0 +1,49 @@
+"Empty SQL query returns 400 empty query error":
+  - skip:
+      features:
+        - headers
+  # Before the fix the empty statement built a null AST and failed with an internal
+  # NullPointerException. catch: bad_request asserts the HTTP status code is exactly 400.
+  # The query is now rejected as empty, and it does not fall back to the legacy engine.
+  - do:
+      catch: bad_request
+      headers:
+        Content-Type: 'application/json'
+      sql:
+        body:
+          query: ""
+  - match: { "$body": "/IllegalArgumentException/" }
+  - match: { "$body": "/Query\\s+is\\s+empty/" }
+  - match: { "$body": "/^(?s)(?!.*NullPointerException)/" }
+
+---
+"Comment and semicolon only SQL query returns 400 empty query error":
+  - skip:
+      features:
+        - headers
+  - do:
+      catch: bad_request
+      headers:
+        Content-Type: 'application/json'
+      sql:
+        body:
+          query: "  /* nothing here */ ;  "
+  - match: { "$body": "/IllegalArgumentException/" }
+  - match: { "$body": "/Query\\s+is\\s+empty/" }
+  - match: { "$body": "/^(?s)(?!.*NullPointerException)/" }
+
+---
+"Empty SQL explain returns 400 empty query error":
+  - skip:
+      features:
+        - headers
+  - do:
+      catch: bad_request
+      headers:
+        Content-Type: 'application/json'
+      sql.explain:
+        body:
+          query: ""
+  - match: { "$body": "/IllegalArgumentException/" }
+  - match: { "$body": "/Query\\s+is\\s+empty/" }
+  - match: { "$body": "/^(?s)(?!.*NullPointerException)/" }

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

@@ -23,6 +23,14 @@ public class AstStatementBuilder extends OpenSearchSQLParserBaseVisitor<Statemen
 
   private final StatementBuilderContext context;
 
+  @Override
+  public Statement visitRoot(OpenSearchSQLParser.RootContext ctx) {
+    if (ctx.sqlStatement() == null) {
+      throw new IllegalArgumentException("Query is empty");
+    }
+    return super.visitRoot(ctx);
+  }
+
   @Override
   public Statement visitSqlStatement(OpenSearchSQLParser.SqlStatementContext ctx) {
     Query query = new Query(astBuilder.visit(ctx), context.fetchSize, SQL);

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

@@ -0,0 +1,50 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.sql.parser;
+
+import static java.util.Collections.emptyList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.opensearch.sql.ast.dsl.AstDSL.alias;
+import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral;
+import static org.opensearch.sql.ast.dsl.AstDSL.project;
+import static org.opensearch.sql.ast.dsl.AstDSL.values;
+import static org.opensearch.sql.executor.QueryType.SQL;
+
+import org.junit.jupiter.api.Test;
+import org.opensearch.sql.ast.statement.Query;
+import org.opensearch.sql.ast.statement.Statement;
+import org.opensearch.sql.sql.antlr.SQLSyntaxParser;
+
+class AstStatementBuilderTest {
+
+  private final SQLSyntaxParser parser = new SQLSyntaxParser();
+
+  @Test
+  public void queryWithoutStatementShouldFail() {
+    for (String query : new String[] {"", " \n\t ", "/* only a comment */", "-- line", ";"}) {
+      IllegalArgumentException e =
+          assertThrows(IllegalArgumentException.class, () -> buildStatement(query));
+      assertEquals("Query is empty", e.getMessage(), "[" + query + "]");
+    }
+  }
+
+  @Test
+  public void canBuildStatementWithSurroundingCommentsAndSemiColon() {
+    assertEquals(
+        new Query(project(values(emptyList()), alias("123", intLiteral(123))), 0, SQL),
+        buildStatement("/* leading */ SELECT 123 -- trailing\n;"));
+  }
+
+  private Statement buildStatement(String query) {
+    return parser
+        .parse(query)
+        .accept(
+            new AstStatementBuilder(
+                new AstBuilder(query),
+                AstStatementBuilder.StatementBuilderContext.builder().build()));
+  }
+}