fix(sql): Report invalid-query errors as client errors (#5532)

UnifiedQueryPlanner.plan() let field-not-found (ErrorReport) and Calcite
validation errors such as table-not-found (CalciteException) fall through
to the catch-all and rethrew them as IllegalStateException. Propagate
ErrorReport unwrapped, map CalciteException to SemanticCheckException, and
log client errors at WARN. Also unwrap ErrorReport to its cause in the SQL
error formatter so the reported type reflects the cause, not the wrapper.

Signed-off-by: Chen Dai <daichen@amazon.com>

Author: dai-chen

api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java

@@ -14,6 +14,7 @@
 import org.apache.calcite.rel.RelRoot;
 import org.apache.calcite.rel.core.Sort;
 import org.apache.calcite.rel.logical.LogicalSort;
+import org.apache.calcite.runtime.CalciteException;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.util.SqlVisitor;
@@ -25,6 +26,7 @@
 import org.opensearch.sql.ast.tree.UnresolvedPlan;
 import org.opensearch.sql.calcite.CalciteRelNodeVisitor;
 import org.opensearch.sql.common.antlr.SyntaxCheckException;
+import org.opensearch.sql.common.error.ErrorReport;
 import org.opensearch.sql.exception.QueryEngineException;
 import org.opensearch.sql.exception.SemanticCheckException;
 
@@ -74,13 +76,18 @@ public RelNode plan(String query) {
     } catch (SyntaxCheckException
         | QueryEngineException
         | UnsupportedOperationException
-        | IllegalArgumentException e) {
-      LOG.error("Failed to plan query: {}", e.getMessage());
+        | IllegalArgumentException
+        | ErrorReport e) {
+      LOG.warn("Failed to plan query: {}", e.getMessage());
       throw e;
+    } catch (CalciteException e) {
+      // Calcite validation errors (e.g. table not found) indicate an invalid query.
+      LOG.warn("Failed to plan query, invalid query: {}", e.getMessage());
+      throw new SemanticCheckException(e.getMessage(), e);
     } catch (AssertionError e) {
       // Calcite throws assertion error directly when building bad RelNode
       String message = "Failed to plan query: invalid plan structure";
-      LOG.error(message, e);
+      LOG.warn(message, e);
       throw new SemanticCheckException(message, e);
     } catch (Exception e) {
       String message = "Failed to plan query: unexpected error";

api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerTest.java

@@ -10,10 +10,12 @@
 
 import java.util.Map;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.runtime.CalciteException;
 import org.apache.calcite.schema.Schema;
 import org.apache.calcite.schema.impl.AbstractSchema;
 import org.junit.Test;
 import org.opensearch.sql.common.antlr.SyntaxCheckException;
+import org.opensearch.sql.common.error.ErrorReport;
 import org.opensearch.sql.exception.SemanticCheckException;
 import org.opensearch.sql.executor.QueryType;
 
@@ -72,7 +74,7 @@ public void testPPLQueryPlanningWithDefaultNamespaceMultiLevel() {
 
     // This is valid in SparkSQL, but Calcite requires "catalog" as the default root schema to
     // resolve it
-    assertThrows(IllegalStateException.class, () -> planner.plan("source = opensearch.employees"));
+    assertThrows(SemanticCheckException.class, () -> planner.plan("source = opensearch.employees"));
   }
 
   @Test
@@ -131,6 +133,20 @@ public void semanticErrorIsRethrownAsSemanticCheckException() {
         .assertErrorMessageEquals("Source and target patterns have different wildcard counts");
   }
 
+  @Test
+  public void fieldNotFoundIsRethrownAsErrorReport() {
+    givenInvalidQuery("source = catalog.employees | where unknown_field = 1")
+        .assertErrorType(ErrorReport.class)
+        .assertErrorMessageContains("Field [unknown_field] not found");
+  }
+
+  @Test
+  public void invalidTableIsRethrownAsSemanticCheckException() {
+    givenInvalidQuery("source = catalog.nonexistent_table")
+        .assertErrorType(SemanticCheckException.class)
+        .assertCauseType(CalciteException.class);
+  }
+
   @Test
   public void assertionErrorIsWrappedAsSemanticCheckException() {
     // Remove when the underlying Calcite assertion is fixed.

legacy/src/main/java/org/opensearch/sql/legacy/executor/format/ErrorMessageFactory.java

@@ -6,6 +6,7 @@
 package org.opensearch.sql.legacy.executor.format;
 
 import org.opensearch.OpenSearchException;
+import org.opensearch.sql.common.error.ErrorReport;
 
 public class ErrorMessageFactory {
   /**
@@ -25,6 +26,10 @@ public static ErrorMessage createErrorMessage(Exception e, int status) {
       OpenSearchException exception = (OpenSearchException) unwrapCause(e);
       return new OpenSearchErrorMessage(exception, exception.status().getStatus());
     }
+    // Unwrap ErrorReport so the error type reflects the underlying cause, not the wrapper.
+    if (e instanceof ErrorReport) {
+      return new ErrorMessage(((ErrorReport) e).getCause(), status);
+    }
     return new ErrorMessage(e, status);
   }
 

legacy/src/test/java/org/opensearch/sql/legacy/unittest/ErrorMessageFactoryTest.java

@@ -9,6 +9,7 @@
 import org.junit.Test;
 import org.opensearch.OpenSearchException;
 import org.opensearch.core.rest.RestStatus;
+import org.opensearch.sql.common.error.ErrorReport;
 import org.opensearch.sql.legacy.executor.format.ErrorMessage;
 import org.opensearch.sql.legacy.executor.format.ErrorMessageFactory;
 import org.opensearch.sql.legacy.executor.format.OpenSearchErrorMessage;
@@ -42,6 +43,25 @@ public void nonOpenSearchExceptionWithWrappedEsExceptionCauseShouldCreateEsError
     Assert.assertTrue(msg instanceof OpenSearchErrorMessage);
   }
 
+  @Test
+  public void errorReportShouldRenderUnderlyingCauseType() {
+    Exception exception =
+        ErrorReport.wrap(new IllegalArgumentException("Field [x] not found.")).build();
+    ErrorMessage msg =
+        ErrorMessageFactory.createErrorMessage(exception, RestStatus.BAD_REQUEST.getStatus());
+    Assert.assertFalse(msg instanceof OpenSearchErrorMessage);
+    Assert.assertTrue(msg.toString().contains("IllegalArgumentException"));
+    Assert.assertFalse(msg.toString().contains("ErrorReport"));
+  }
+
+  @Test
+  public void errorReportWrappingEsExceptionShouldCreateEsErrorMessage() {
+    Exception exception = ErrorReport.wrap(new OpenSearchException(nonOpenSearchThrowable)).build();
+    ErrorMessage msg =
+        ErrorMessageFactory.createErrorMessage(exception, RestStatus.NOT_FOUND.getStatus());
+    Assert.assertTrue(msg instanceof OpenSearchErrorMessage);
+  }
+
   @Test
   public void
       nonOpenSearchExceptionWithMultiLayerWrappedEsExceptionCauseShouldCreateEsErrorMessage() {