diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java index f51ffabdc35..d2813f6eb17 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java @@ -11,6 +11,7 @@ import java.io.IOException; import org.json.JSONObject; import org.junit.jupiter.api.Test; +import org.opensearch.client.Request; import org.opensearch.client.ResponseException; import org.opensearch.sql.ppl.PPLIntegTestCase; @@ -214,4 +215,36 @@ public void testStageDescriptionIsUserFriendly() throws IOException { || stageDescription.toLowerCase().contains("run") || stageDescription.toLowerCase().contains("query")); } + + // An alias field whose path targets a text multi-field (e.g. "source.keyword") is not present in + // the flattened mapping. It used to surface an opaque NullPointerException; it must now report a + // structured FIELD_NOT_FOUND error with a suggestion. + @Test + public void testAliasToUnresolvablePathIncludesStructuredError() throws IOException { + String index = "test_alias_unresolved_keyword"; + Request createIndex = new Request("PUT", "/" + index); + createIndex.setJsonEntity( + "{ \"mappings\": { \"properties\": {" + + " \"source\": { \"type\": \"text\", \"fields\": { \"keyword\": { \"type\":" + + " \"keyword\" } } }," + + " \"source_alias\": { \"type\": \"alias\", \"path\": \"source.keyword\" } } } }"); + client().performRequest(createIndex); + + ResponseException exception = + assertThrows(ResponseException.class, () -> executeQuery("source=" + index)); + + JSONObject error = + new JSONObject(getResponseBody(exception.getResponse())).getJSONObject("error"); + + assertEquals("FIELD_NOT_FOUND", error.getString("code")); + assertTrue( + "Details should name the alias field and path", + error + .getString("details") + .contains("Alias field [source_alias] refers to unresolved path [source.keyword]")); + JSONObject context = error.getJSONObject("context"); + assertEquals("source_alias", context.getString("alias_field")); + assertEquals("source.keyword", context.getString("alias_path")); + assertTrue("Should include a suggestion", error.has("suggestion")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/QueryValidationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/QueryValidationIT.java index 3373b46303c..71f16f30e43 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/QueryValidationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/QueryValidationIT.java @@ -103,6 +103,31 @@ private static void whenExecuteMalformedPayload() throws IOException { client().performRequest(request); } + // An alias field whose path targets a text multi-field (e.g. "source.keyword") must fail with a + // descriptive client error rather than an opaque NullPointerException. + @Test + public void testAliasToKeywordMultiFieldFailsWithBadRequest() throws IOException { + String index = "test_alias_unresolved_keyword"; + createIndexWithMapping( + index, + "{ \"properties\": {" + + " \"source\": { \"type\": \"text\", \"fields\": { \"keyword\": { \"type\":" + + " \"keyword\" } } }," + + " \"source_alias\": { \"type\": \"alias\", \"path\": \"source.keyword\" } } }"); + + expectResponseException() + .hasStatusCode(BAD_REQUEST) + .hasErrorType("ErrorReport") + .containsMessage("Alias field [source_alias] refers to unresolved path [source.keyword]") + .whenExecute(String.format(Locale.ROOT, "SELECT * FROM %s", index)); + } + + private static void createIndexWithMapping(String indexName, String mapping) throws IOException { + Request request = new Request("PUT", "/" + indexName); + request.setJsonEntity(String.format(Locale.ROOT, "{ \"mappings\": %s }", mapping)); + client().performRequest(request); + } + public ResponseExceptionAssertion expectResponseException() { return new ResponseExceptionAssertion(exceptionRule); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java index 79d49a143de..76de0c30a08 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java @@ -14,8 +14,12 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import org.apache.commons.lang3.EnumUtils; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.common.error.QueryProcessingStage; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.SemanticCheckException; /** The extension of ExprType in OpenSearch. */ @EqualsAndHashCode @@ -297,7 +301,26 @@ private static void validateAliasType(Map result) { (key, value) -> { if (value instanceof OpenSearchAliasType && value.getOriginalPath().isPresent()) { String originalPath = value.getOriginalPath().get(); - result.put(key, new OpenSearchAliasType(originalPath, result.get(originalPath))); + OpenSearchDataType target = result.get(originalPath); + if (target == null) { + throw ErrorReport.wrap( + new SemanticCheckException( + String.format( + "Alias field [%s] refers to unresolved path [%s].", + key, originalPath))) + .code(ErrorCode.FIELD_NOT_FOUND) + .stage(QueryProcessingStage.ANALYZING) + .location("while resolving alias fields in the index mapping") + .context("alias_field", key) + .context("alias_path", originalPath) + .suggestion( + "The alias path must point to an existing field in the mapping; a text" + + " multi-field (e.g. \"" + + originalPath + + ".keyword\") or a removed/renamed field is not a valid alias target.") + .build(); + } + result.put(key, new OpenSearchAliasType(originalPath, target)); } }); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java index 40985130c52..1479ccfb615 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java @@ -41,8 +41,12 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.common.error.QueryProcessingStage; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.SemanticCheckException; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) class OpenSearchDataTypeTest { @@ -483,6 +487,53 @@ public void test_AliasType() { () -> assertEquals("original_path2", aliasTypeOnDouble.getOriginalPath().orElseThrow())); } + @Test + public void traverseAndFlatten_alias_to_unresolvable_path_throws_descriptive_error() { + // Alias path targets a text multi-field, which is not in the flattened mapping. + Map keywordAliasTree = + Map.of( + "source", + textKeywordType, + "source_alias", + new OpenSearchAliasType("source.keyword", OpenSearchDataType.of(MappingType.Invalid))); + ErrorReport keywordError = + assertThrows( + ErrorReport.class, () -> OpenSearchDataType.traverseAndFlatten(keywordAliasTree)); + assertAll( + () -> assertEquals(ErrorCode.FIELD_NOT_FOUND, keywordError.getCode()), + () -> assertEquals(QueryProcessingStage.ANALYZING, keywordError.getStage()), + () -> assertTrue(keywordError.getCause() instanceof SemanticCheckException), + () -> + assertEquals( + "Alias field [source_alias] refers to unresolved path [source.keyword].", + keywordError.getCause().getMessage()), + () -> assertEquals("source_alias", keywordError.getContext().get("alias_field")), + () -> assertEquals("source.keyword", keywordError.getContext().get("alias_path")), + () -> + assertTrue( + keywordError.getSuggestion().contains("\"source.keyword.keyword\"") + && keywordError.getSuggestion().contains("not a valid alias target"))); + + // Alias path targets a field that does not exist. + Map missingFieldTree = + Map.of( + "col1", + textType, + "col_alias", + new OpenSearchAliasType("missing", OpenSearchDataType.of(MappingType.Invalid))); + ErrorReport missingError = + assertThrows( + ErrorReport.class, () -> OpenSearchDataType.traverseAndFlatten(missingFieldTree)); + assertAll( + () -> assertEquals(ErrorCode.FIELD_NOT_FOUND, missingError.getCode()), + () -> assertTrue(missingError.getCause() instanceof SemanticCheckException), + () -> + assertEquals( + "Alias field [col_alias] refers to unresolved path [missing].", + missingError.getCause().getMessage()), + () -> assertEquals("missing", missingError.getContext().get("alias_path"))); + } + @Test public void test_parseMapping_on_AliasType() { Map indexMapping1 =