Skip to content

Commit 7f31510

Browse files
authored
fix(integ): repair two pre-existing IT failures on main (opensearch-project#5545)
QueryValidationIT.testAliasToKeywordMultiFieldFailsWithBadRequest: opensearch-project#5532 changed the SQL error formatter to unwrap ErrorReport to its cause, so the reported type is now the underlying SemanticCheckException, not the ErrorReport wrapper. Update the expected error type to match. Query is still correctly rejected (400); only the envelope type changed. CalciteExplainIT.testNoMvBasic / testNoMvWithEval: the assertion scanned the full logical+physical explain YAML for ARRAY_JOIN, but the physical section renders differently with pushdown disabled (CalciteNoPushdownIT), making the test flaky on some runners. nomv lowers to MVJOIN -> ARRAY_JOIN, which is stable in the LOGICAL plan regardless of pushdown (verified with pushdown on and off). Assert on the logical section only. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 370c230 commit 7f31510

2 files changed

Lines changed: 27 additions & 7 deletions

File tree

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2888,10 +2888,13 @@ public void testNoMvBasic() throws IOException {
28882888
"source=%s | fields firstname, age | eval names = array(firstname) | nomv names |"
28892889
+ " fields names",
28902890
TEST_INDEX_BANK);
2891-
var result = explainQueryYaml(query);
2891+
// Assert on the LOGICAL plan only: nomv lowers to MVJOIN -> ARRAY_JOIN, which is stable in the
2892+
// logical plan regardless of pushdown. The physical section's rendering varies with the
2893+
// pushdown setting (e.g. under CalciteNoPushdownIT), so scanning the whole output is flaky.
2894+
String logical = logicalPlan(explainQueryYaml(query));
28922895
Assert.assertTrue(
2893-
"Expected explain to contain ARRAY_JOIN function",
2894-
result.toLowerCase().contains("array_join"));
2896+
"Expected logical plan to contain ARRAY_JOIN function",
2897+
logical.toLowerCase().contains("array_join"));
28952898
}
28962899

28972900
@Test
@@ -2901,10 +2904,24 @@ public void testNoMvWithEval() throws IOException {
29012904
"source=%s | eval full_name = concat(firstname, ' J.') | eval name_array ="
29022905
+ " array(full_name) | nomv name_array | fields name_array",
29032906
TEST_INDEX_BANK);
2904-
var result = explainQueryYaml(query);
2907+
String logical = logicalPlan(explainQueryYaml(query)).toLowerCase();
29052908
Assert.assertTrue(
2906-
"Expected explain to contain both CONCAT and ARRAY_JOIN",
2907-
result.toLowerCase().contains("concat") && result.toLowerCase().contains("array_join"));
2909+
"Expected logical plan to contain both CONCAT and ARRAY_JOIN",
2910+
logical.contains("concat") && logical.contains("array_join"));
2911+
}
2912+
2913+
/**
2914+
* Return just the {@code logical:} section of a YAML explain result (everything before the {@code
2915+
* physical:} key). The logical plan is deterministic across pushdown on/off, whereas the physical
2916+
* section's rendering varies — so assertions on plan content should target the logical plan to
2917+
* avoid flakiness.
2918+
*
2919+
* <p>Splits on the line-anchored top-level {@code "\nphysical:"} key so a string value that
2920+
* happens to contain {@code "physical:"} inside the logical section can't truncate the plan.
2921+
*/
2922+
private static String logicalPlan(String explainYaml) {
2923+
int physicalIdx = explainYaml.indexOf("\nphysical:");
2924+
return physicalIdx >= 0 ? explainYaml.substring(0, physicalIdx) : explainYaml;
29082925
}
29092926

29102927
@Test

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,10 @@ public void testAliasToKeywordMultiFieldFailsWithBadRequest() throws IOException
117117

118118
expectResponseException()
119119
.hasStatusCode(BAD_REQUEST)
120-
.hasErrorType("ErrorReport")
120+
// #5532 unwraps ErrorReport to its cause in the SQL error formatter, so the reported type
121+
// is
122+
// now the underlying SemanticCheckException, not the ErrorReport wrapper.
123+
.hasErrorType("SemanticCheckException")
121124
.containsMessage("Alias field [source_alias] refers to unresolved path [source.keyword]")
122125
.whenExecute(String.format(Locale.ROOT, "SELECT * FROM %s", index));
123126
}

0 commit comments

Comments
 (0)