Skip to content

Commit 30af8b2

Browse files
RyanL1997RyanL1997
authored
[BugFix] Handle opaque NullPointerException for unresolvable alias-type field path (#5536)
* Fix opaque NullPointerException for unresolvable alias-type field path When a mapping contains a field of "type": "alias" whose "path" points to a target absent from the flattened mapping (a text multi-field such as field.keyword, or a removed/renamed field), validateAliasType passed a null target into the OpenSearchAliasType constructor, which dereferenced it at super(type.getExprCoreType()) and surfaced an opaque NullPointerException. Guard the null target and throw a SemanticCheckException naming the alias field and its unresolved path. SemanticCheckException extends QueryEngineException, so JdbcResponseFormatter maps it to HTTP 400 (client error) rather than the misleading 500 a generic exception would produce. Add unit tests covering the .keyword multi-field and missing-field cases. Fixes #5535 Signed-off-by: Jialiang Liang <jiallian@amazon.com> * Add integration test and trim unit-test comments for alias path fix Add a QueryValidationIT case asserting that SELECT * over an index whose alias field targets a text multi-field (source.keyword) returns a 400 SemanticCheckException with the descriptive message. An alias pointing at a truly missing field is rejected by OpenSearch at index-creation time, so it is not reachable through the SQL plugin and is covered by the unit test only. Shorten the unit-test comments and drop inline issue references. Signed-off-by: Jialiang Liang <jiallian@amazon.com> * Apply spotless formatting Signed-off-by: Jialiang Liang <jiallian@amazon.com> * Adopt ErrorReport for unresolvable alias path error Wrap the SemanticCheckException in an ErrorReport (the report-builder interface from #5266) so the error carries structured context as it bubbles up: FIELD_NOT_FOUND code, ANALYZING stage, a location chain, the alias field and path as context, and a fix suggestion. On the PPL/Calcite path this renders as a rich structured error; on the SQL JDBC path it still returns a clear 400 (RestSqlAction unwraps to the SemanticCheckException cause), though the JdbcResponseFormatter does not yet render the ErrorReport structure. Update the unit test to assert the ErrorReport code/stage/context/cause, the SQL IT for the ErrorReport type, and add a PPL IT asserting the structured FIELD_NOT_FOUND error in CalciteErrorReportStageIT. Signed-off-by: Jialiang Liang <jiallian@amazon.com> --------- Signed-off-by: Jialiang Liang <jiallian@amazon.com>
1 parent 8394e5c commit 30af8b2

4 files changed

Lines changed: 133 additions & 1 deletion

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.io.IOException;
1212
import org.json.JSONObject;
1313
import org.junit.jupiter.api.Test;
14+
import org.opensearch.client.Request;
1415
import org.opensearch.client.ResponseException;
1516
import org.opensearch.sql.ppl.PPLIntegTestCase;
1617

@@ -214,4 +215,36 @@ public void testStageDescriptionIsUserFriendly() throws IOException {
214215
|| stageDescription.toLowerCase().contains("run")
215216
|| stageDescription.toLowerCase().contains("query"));
216217
}
218+
219+
// An alias field whose path targets a text multi-field (e.g. "source.keyword") is not present in
220+
// the flattened mapping. It used to surface an opaque NullPointerException; it must now report a
221+
// structured FIELD_NOT_FOUND error with a suggestion.
222+
@Test
223+
public void testAliasToUnresolvablePathIncludesStructuredError() throws IOException {
224+
String index = "test_alias_unresolved_keyword";
225+
Request createIndex = new Request("PUT", "/" + index);
226+
createIndex.setJsonEntity(
227+
"{ \"mappings\": { \"properties\": {"
228+
+ " \"source\": { \"type\": \"text\", \"fields\": { \"keyword\": { \"type\":"
229+
+ " \"keyword\" } } },"
230+
+ " \"source_alias\": { \"type\": \"alias\", \"path\": \"source.keyword\" } } } }");
231+
client().performRequest(createIndex);
232+
233+
ResponseException exception =
234+
assertThrows(ResponseException.class, () -> executeQuery("source=" + index));
235+
236+
JSONObject error =
237+
new JSONObject(getResponseBody(exception.getResponse())).getJSONObject("error");
238+
239+
assertEquals("FIELD_NOT_FOUND", error.getString("code"));
240+
assertTrue(
241+
"Details should name the alias field and path",
242+
error
243+
.getString("details")
244+
.contains("Alias field [source_alias] refers to unresolved path [source.keyword]"));
245+
JSONObject context = error.getJSONObject("context");
246+
assertEquals("source_alias", context.getString("alias_field"));
247+
assertEquals("source.keyword", context.getString("alias_path"));
248+
assertTrue("Should include a suggestion", error.has("suggestion"));
249+
}
217250
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,31 @@ private static void whenExecuteMalformedPayload() throws IOException {
103103
client().performRequest(request);
104104
}
105105

106+
// An alias field whose path targets a text multi-field (e.g. "source.keyword") must fail with a
107+
// descriptive client error rather than an opaque NullPointerException.
108+
@Test
109+
public void testAliasToKeywordMultiFieldFailsWithBadRequest() throws IOException {
110+
String index = "test_alias_unresolved_keyword";
111+
createIndexWithMapping(
112+
index,
113+
"{ \"properties\": {"
114+
+ " \"source\": { \"type\": \"text\", \"fields\": { \"keyword\": { \"type\":"
115+
+ " \"keyword\" } } },"
116+
+ " \"source_alias\": { \"type\": \"alias\", \"path\": \"source.keyword\" } } }");
117+
118+
expectResponseException()
119+
.hasStatusCode(BAD_REQUEST)
120+
.hasErrorType("ErrorReport")
121+
.containsMessage("Alias field [source_alias] refers to unresolved path [source.keyword]")
122+
.whenExecute(String.format(Locale.ROOT, "SELECT * FROM %s", index));
123+
}
124+
125+
private static void createIndexWithMapping(String indexName, String mapping) throws IOException {
126+
Request request = new Request("PUT", "/" + indexName);
127+
request.setJsonEntity(String.format(Locale.ROOT, "{ \"mappings\": %s }", mapping));
128+
client().performRequest(request);
129+
}
130+
106131
public ResponseExceptionAssertion expectResponseException() {
107132
return new ResponseExceptionAssertion(exceptionRule);
108133
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414
import lombok.EqualsAndHashCode;
1515
import lombok.Getter;
1616
import org.apache.commons.lang3.EnumUtils;
17+
import org.opensearch.sql.common.error.ErrorCode;
18+
import org.opensearch.sql.common.error.ErrorReport;
19+
import org.opensearch.sql.common.error.QueryProcessingStage;
1720
import org.opensearch.sql.data.type.ExprCoreType;
1821
import org.opensearch.sql.data.type.ExprType;
22+
import org.opensearch.sql.exception.SemanticCheckException;
1923

2024
/** The extension of ExprType in OpenSearch. */
2125
@EqualsAndHashCode
@@ -297,7 +301,26 @@ private static void validateAliasType(Map<String, OpenSearchDataType> result) {
297301
(key, value) -> {
298302
if (value instanceof OpenSearchAliasType && value.getOriginalPath().isPresent()) {
299303
String originalPath = value.getOriginalPath().get();
300-
result.put(key, new OpenSearchAliasType(originalPath, result.get(originalPath)));
304+
OpenSearchDataType target = result.get(originalPath);
305+
if (target == null) {
306+
throw ErrorReport.wrap(
307+
new SemanticCheckException(
308+
String.format(
309+
"Alias field [%s] refers to unresolved path [%s].",
310+
key, originalPath)))
311+
.code(ErrorCode.FIELD_NOT_FOUND)
312+
.stage(QueryProcessingStage.ANALYZING)
313+
.location("while resolving alias fields in the index mapping")
314+
.context("alias_field", key)
315+
.context("alias_path", originalPath)
316+
.suggestion(
317+
"The alias path must point to an existing field in the mapping; a text"
318+
+ " multi-field (e.g. \""
319+
+ originalPath
320+
+ ".keyword\") or a removed/renamed field is not a valid alias target.")
321+
.build();
322+
}
323+
result.put(key, new OpenSearchAliasType(originalPath, target));
301324
}
302325
});
303326
}

‎opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,12 @@
4141
import org.junit.jupiter.params.provider.Arguments;
4242
import org.junit.jupiter.params.provider.EnumSource;
4343
import org.junit.jupiter.params.provider.MethodSource;
44+
import org.opensearch.sql.common.error.ErrorCode;
45+
import org.opensearch.sql.common.error.ErrorReport;
46+
import org.opensearch.sql.common.error.QueryProcessingStage;
4447
import org.opensearch.sql.data.type.ExprCoreType;
4548
import org.opensearch.sql.data.type.ExprType;
49+
import org.opensearch.sql.exception.SemanticCheckException;
4650

4751
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
4852
class OpenSearchDataTypeTest {
@@ -483,6 +487,53 @@ public void test_AliasType() {
483487
() -> assertEquals("original_path2", aliasTypeOnDouble.getOriginalPath().orElseThrow()));
484488
}
485489

490+
@Test
491+
public void traverseAndFlatten_alias_to_unresolvable_path_throws_descriptive_error() {
492+
// Alias path targets a text multi-field, which is not in the flattened mapping.
493+
Map<String, OpenSearchDataType> keywordAliasTree =
494+
Map.of(
495+
"source",
496+
textKeywordType,
497+
"source_alias",
498+
new OpenSearchAliasType("source.keyword", OpenSearchDataType.of(MappingType.Invalid)));
499+
ErrorReport keywordError =
500+
assertThrows(
501+
ErrorReport.class, () -> OpenSearchDataType.traverseAndFlatten(keywordAliasTree));
502+
assertAll(
503+
() -> assertEquals(ErrorCode.FIELD_NOT_FOUND, keywordError.getCode()),
504+
() -> assertEquals(QueryProcessingStage.ANALYZING, keywordError.getStage()),
505+
() -> assertTrue(keywordError.getCause() instanceof SemanticCheckException),
506+
() ->
507+
assertEquals(
508+
"Alias field [source_alias] refers to unresolved path [source.keyword].",
509+
keywordError.getCause().getMessage()),
510+
() -> assertEquals("source_alias", keywordError.getContext().get("alias_field")),
511+
() -> assertEquals("source.keyword", keywordError.getContext().get("alias_path")),
512+
() ->
513+
assertTrue(
514+
keywordError.getSuggestion().contains("\"source.keyword.keyword\"")
515+
&& keywordError.getSuggestion().contains("not a valid alias target")));
516+
517+
// Alias path targets a field that does not exist.
518+
Map<String, OpenSearchDataType> missingFieldTree =
519+
Map.of(
520+
"col1",
521+
textType,
522+
"col_alias",
523+
new OpenSearchAliasType("missing", OpenSearchDataType.of(MappingType.Invalid)));
524+
ErrorReport missingError =
525+
assertThrows(
526+
ErrorReport.class, () -> OpenSearchDataType.traverseAndFlatten(missingFieldTree));
527+
assertAll(
528+
() -> assertEquals(ErrorCode.FIELD_NOT_FOUND, missingError.getCode()),
529+
() -> assertTrue(missingError.getCause() instanceof SemanticCheckException),
530+
() ->
531+
assertEquals(
532+
"Alias field [col_alias] refers to unresolved path [missing].",
533+
missingError.getCause().getMessage()),
534+
() -> assertEquals("missing", missingError.getContext().get("alias_path")));
535+
}
536+
486537
@Test
487538
public void test_parseMapping_on_AliasType() {
488539
Map<String, Object> indexMapping1 =

0 commit comments

Comments
 (0)