Misc. refactoring after code self-review

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

Author: Swiddis

common/src/main/java/org/opensearch/sql/common/error/ErrorReport.java

@@ -189,7 +189,8 @@ public static class Builder {
     private Builder(Exception cause) {
       this.cause = cause;
       // Default details to the original exception message
-      this.details = cause.getLocalizedMessage();
+      this.details =
+          cause.getLocalizedMessage() != null ? cause.getLocalizedMessage() : cause.getMessage();
     }
 
     /** Set the machine-readable error code. */
@@ -200,7 +201,10 @@ public Builder code(ErrorCode code) {
 
     /** Set the query processing stage where the error occurred. */
     public Builder stage(QueryProcessingStage stage) {
-      this.stage = stage;
+      // Don't overwrite more-specific stages with less-specific ones
+      if (this.stage == null) {
+        this.stage = stage;
+      }
       return this;
     }
 

common/src/main/java/org/opensearch/sql/common/error/QueryProcessingStage.java

@@ -5,37 +5,20 @@
 
 package org.opensearch.sql.common.error;
 
+import lombok.Getter;
+
 /**
  * Enumeration of query processing stages for error location tracking. These stages represent the
- * major phases of query execution in the Calcite query planner.
- *
- * <p>Stage progression for a typical query: PARSING → AST_BUILDING → ANALYZING → OPTIMIZING →
- * PLAN_CONVERSION → EXECUTING → RESULT_PROCESSING
+ * major phases of query execution in the Calcite query planner. May not be a complete list, add
+ * stages if needed.
  */
+@Getter
 public enum QueryProcessingStage {
-  /**
-   * PARSING stage: PPL/SQL syntax parsing via ANTLR lexer/parser. Errors: Syntax errors, unexpected
-   * tokens, malformed queries.
-   */
-  PARSING("Validating query syntax"),
-
-  /**
-   * AST_BUILDING stage: Abstract Syntax Tree construction from parse tree. Errors: Invalid AST
-   * structure, node creation failures.
-   */
-  AST_BUILDING("Assembling the query steps"),
-
   /**
    * ANALYZING stage: Semantic validation and type checking. Errors: Field not found, type
    * mismatches, semantic violations.
    */
-  ANALYZING("Validating the query"),
-
-  /**
-   * OPTIMIZING stage: Logical plan optimization with transformation rules. Errors: Optimization
-   * failures, rule application errors.
-   */
-  OPTIMIZING("Optimizing the query"),
+  ANALYZING("Parsing and validating the query"),
 
   /**
    * PLAN_CONVERSION stage: Conversion to Calcite execution plan with system limits. Errors:
@@ -47,31 +30,15 @@ public enum QueryProcessingStage {
    * EXECUTING stage: Query execution via OpenSearch engine. Errors: Execution failures, index
    * access errors, resource limits.
    */
-  EXECUTING("Running the query"),
-
-  /**
-   * RESULT_PROCESSING stage: Result formatting and cursor management. Errors: Result formatting
-   * failures, cursor errors.
-   */
-  RESULT_PROCESSING("Processing the query results"),
-
-  /**
-   * CLUSTER_VALIDATION stage: Prevalidation that cluster is healthy before execution. Errors:
-   * Cluster unhealthy, nodes unavailable.
-   */
-  CLUSTER_VALIDATION("Checking cluster health");
+  EXECUTING("Running the query");
 
+  /** -- GETTER -- Get human-readable display name for this stage. */
   private final String displayName;
 
   QueryProcessingStage(String displayName) {
     this.displayName = displayName;
   }
 
-  /** Get human-readable display name for this stage. */
-  public String getDisplayName() {
-    return displayName;
-  }
-
   /** Get lowercase name suitable for JSON serialization. */
   public String toJsonKey() {
     return name().toLowerCase();

common/src/main/java/org/opensearch/sql/common/error/StageErrorHandler.java

@@ -37,9 +37,6 @@ public static <T> T executeStage(
       QueryProcessingStage stage, Supplier<T> operation, String location) {
     try {
       return operation.get();
-    } catch (ErrorReport e) {
-      // If already an ErrorReport, just add stage if not set
-      throw ErrorReport.wrap(e).stage(stage).location(location).build();
     } catch (Exception e) {
       throw ErrorReport.wrap(e).stage(stage).location(location).build();
     }

common/src/test/java/org/opensearch/sql/common/error/ErrorReportTest.java

@@ -8,6 +8,8 @@
 import static org.junit.jupiter.api.Assertions.*;
 
 import java.util.Map;
+import org.hamcrest.CoreMatchers;
+import org.hamcrest.MatcherAssert;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for ErrorReport. */
@@ -64,7 +66,7 @@ public void testErrorReportJsonMapWithStageInContext() {
     @SuppressWarnings("unchecked")
     Map<String, Object> context = (Map<String, Object>) json.get("context");
     assertEquals("analyzing", context.get("stage"));
-    assertEquals("Validating the query", context.get("stage_description"));
+    assertEquals("Parsing and validating the query", context.get("stage_description"));
     assertEquals("test", context.get("field_name"));
   }
 
@@ -140,11 +142,11 @@ public void testToDetailedMessage() {
 
     String message = report.toDetailedMessage();
 
-    assertTrue(message.contains("FIELD_NOT_FOUND"));
-    assertTrue(message.contains("Validating the query"));
-    assertTrue(message.contains("Field not found"));
-    assertTrue(message.contains("while resolving fields"));
-    assertTrue(message.contains("field_name"));
-    assertTrue(message.contains("Check field name"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("FIELD_NOT_FOUND"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("validating the query"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("Field not found"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("while resolving fields"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("field_name"));
+    MatcherAssert.assertThat(message, CoreMatchers.containsString("Check field name"));
   }
 }

docs/user/ppl/cmd/mvcombine.md

@@ -124,6 +124,6 @@ source=mvcombine_data
 
 Expected output:
 ```text
-{'context': {'stage': 'analyzing', 'stage_description': 'Validating the query'}, 'details': 'Field [does_not_exist] not found.', 'location': ['while preparing and validating the query plan'], 'code': 'FIELD_NOT_FOUND', 'type': 'IllegalArgumentException'}
+{'context': {'stage': 'analyzing', 'stage_description': 'Parsing and validating the query'}, 'details': 'Field [does_not_exist] not found.', 'location': ['while preparing and validating the query plan'], 'code': 'FIELD_NOT_FOUND', 'type': 'IllegalArgumentException'}
 Error: Query returned no data
 ```

docs/user/ppl/cmd/mvexpand.md

@@ -132,6 +132,6 @@ source=people
 
 Expected output:
 ```text
-{'context': {'stage': 'analyzing', 'stage_description': 'Validating the query', 'command': 'mvexpand'}, 'details': "Field 'tags' not found in the schema", 'location': ['while preparing and validating the query plan', 'while evaluating the input field for mvexpand'], 'code': 'FIELD_NOT_FOUND', 'type': 'SemanticCheckException'}
+{'context': {'stage': 'analyzing', 'stage_description': 'Parsing and validating the query', 'command': 'mvexpand'}, 'details': "Field 'tags' not found in the schema", 'location': ['while preparing and validating the query plan', 'while evaluating the input field for mvexpand'], 'code': 'FIELD_NOT_FOUND', 'type': 'SemanticCheckException'}
 Error: Query returned no data
 ```

docs/user/ppl/cmd/rex.md

@@ -228,7 +228,7 @@ source=accounts
 The query returns the following results:
 
 ```text
-{'context': {'stage': 'analyzing', 'stage_description': 'Validating the query', 'command': 'rex'}, 'details': "Invalid capture group name 'user_name'.", 'location': ['while preparing and validating the query plan', 'while processing the rex command', 'while validating the capture groups for the pattern'], 'code': 'SYNTAX_ERROR', 'type': 'IllegalArgumentException', 'suggestion': 'Java Regex capture groups must be alphanumeric and start with a letter. Update the capture group to be alphanumeric.'}
+{'context': {'stage': 'analyzing', 'stage_description': 'Parsing and validating the query', 'command': 'rex'}, 'details': "Invalid capture group name 'user_name'.", 'location': ['while preparing and validating the query plan', 'while processing the rex command', 'while validating the capture groups for the pattern'], 'code': 'SYNTAX_ERROR', 'type': 'IllegalArgumentException', 'suggestion': 'Java Regex capture groups must be alphanumeric and start with a letter. Update the capture group to be alphanumeric.'}
 Error: Query returned no data
 ```
 

docs/user/ppl/interfaces/protocol.md

@@ -123,7 +123,7 @@ Expected output:
     "context": {
       "stage": "analyzing",
       "index_name": "unknown",
-      "stage_description": "Validating the query"
+      "stage_description": "Parsing and validating the query"
     },
     "details": "no such index [unknown]",
     "location": [

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java

@@ -108,7 +108,7 @@ public void testMultipleFieldErrorsIncludeStage() throws IOException {
   }
 
   @Test
-  public void testErrorReportTypeIsCorrect() throws IOException {
+  public void testErrorReportTypeMatchesExceptionType() throws IOException {
     ResponseException exception =
         assertThrows(
             ResponseException.class,
@@ -126,7 +126,7 @@ public void testErrorReportTypeIsCorrect() throws IOException {
   }
 
   @Test
-  public void testErrorCodePresent() throws IOException {
+  public void testFieldNotFoundIncludesErrorCode() throws IOException {
     ResponseException exception =
         assertThrows(
             ResponseException.class,
@@ -136,12 +136,9 @@ public void testErrorCodePresent() throws IOException {
     JSONObject response = new JSONObject(responseBody);
     JSONObject error = response.getJSONObject("error");
 
-    // Verify error may have code field (optional, but if present should be valid)
-    if (error.has("code")) {
-      String code = error.getString("code");
-      assertFalse("Error code should not be empty", code.isEmpty());
-      assertFalse("Error code should not be UNKNOWN", code.equals("UNKNOWN"));
-    }
+    String code = error.getString("code");
+    assertFalse("Error code should not be empty", code.isEmpty());
+    assertFalse("Error code should not be UNKNOWN", code.equals("UNKNOWN"));
   }
 
   @Test

integ-test/src/test/java/org/opensearch/sql/calcite/standalone/CalcitePPLIntegTestCase.java

@@ -200,8 +200,7 @@ public void onFailure(Exception e) {
               case IllegalArgumentException illegalArgumentException ->
                   // most exceptions thrown by Calcite when resolve a plan.
                   throw illegalArgumentException;
-              case null, default ->
-                  throw new IllegalStateException("Exception happened during execution", e);
+              default -> throw new IllegalStateException("Exception happened during execution", e);
             }
           }
         },