Skip to content

Commit 0786a0b

Browse files
committed
Fix integration test failures after merge with origin/main
Update expected output files (335+ YAML/JSON) to reflect changes from the validation branch's OpenSearchRelToSqlConverter, leastRestrictive() type coercion, and PplTypeCoercionRule. Fix behavioral test changes: - CalcitePPLBasicIT: testBetweenWithIncompatibleTypes now returns results due to VARCHAR-INTEGER coercion support - CalciteBinCommandIT: updated error message assertion - CalciteChartCommandIT: timestamp fields now preserve UDT types - CalciteExplainIT: reverted timechart tests to origin/main versions, updated regex test to use loadExpectedPlan - Added YAML/JSON regeneration mechanism via REGENERATE_YAML env var
1 parent a343251 commit 0786a0b

350 files changed

Lines changed: 1302 additions & 1352 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

integ-test/build.gradle

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,11 @@ task integJdbcTest(type: RestIntegTestTask) {
372372
systemProperty "jdbcFile", System.getProperty("jdbcFile")
373373
}
374374

375+
// Pass through REGENERATE_YAML for expected output regeneration
376+
if (System.getProperty("REGENERATE_YAML") != null) {
377+
systemProperty "REGENERATE_YAML", System.getProperty("REGENERATE_YAML")
378+
}
379+
375380
systemProperty 'tests.security.manager', 'false'
376381
systemProperty('project.root', project.projectDir.absolutePath)
377382

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,10 +1011,7 @@ public void testBinsOnTimeFieldWithPushdownDisabled_ShouldFail() throws IOExcept
10111011
"Expected clear error message about bins parameter requirements on timestamp fields, but"
10121012
+ " got: "
10131013
+ errorMessage,
1014-
// TODO: Fix with https://github.com/opensearch-project/sql/issues/4973
1015-
errorMessage.contains(
1016-
"resolving method 'minus[class java.lang.String, class java.lang.String]' in class"
1017-
+ " class org.apache.calcite.runtime.SqlFunctions"));
1014+
errorMessage.contains("bins' parameter on timestamp fields requires"));
10181015
}
10191016

10201017
@Test

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public void testChartMaxValueOverCategoryByTimestampSpanWeek() throws IOExceptio
130130
verifySchema(
131131
result,
132132
schema("category", "string"),
133-
schema("timestamp", "string"),
133+
schema("timestamp", "timestamp"),
134134
schema("max(value)", "int"));
135135
// All data within same week span
136136
verifyDataRows(
@@ -152,7 +152,7 @@ public void testChartMaxValueByTimestampSpanDayAndWeek() throws IOException {
152152
verifySchema(
153153
result,
154154
schema("timestamp", "timestamp"),
155-
schema("@timestamp", "string"),
155+
schema("@timestamp", "timestamp"),
156156
schema("max(value)", "int"));
157157
// Data grouped by day spans
158158
verifyDataRows(

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -478,42 +478,42 @@ public void testExplainWithTimechartCount() throws IOException {
478478

479479
@Test
480480
public void testExplainTimechartPerSecond() throws IOException {
481-
var result = explainQueryYaml("source=events | timechart span=2m per_second(cpu_usage)");
481+
var result = explainQueryToString("source=events | timechart span=2m per_second(cpu_usage)");
482482
assertTrue(
483483
result.contains(
484-
"per_second(cpu_usage)=[DIVIDE(*($1, 1000.0E0), TIMESTAMPDIFF('MILLISECOND', $0,"
485-
+ " TIMESTAMPADD('MINUTE', 2, $0)))]"));
486-
assertTrue(result.contains("per_second(cpu_usage)=[SUM($1)]"));
484+
"per_second(cpu_usage)=[DIVIDE(*($1, 1000.0E0), TIMESTAMPDIFF('MILLISECOND':VARCHAR,"
485+
+ " $0, TIMESTAMPADD('MINUTE':VARCHAR, 2, $0)))]"));
486+
assertTrue(result.contains("per_second(cpu_usage)=[SUM($0)]"));
487487
}
488488

489489
@Test
490490
public void testExplainTimechartPerMinute() throws IOException {
491-
var result = explainQueryYaml("source=events | timechart span=2m per_minute(cpu_usage)");
491+
var result = explainQueryToString("source=events | timechart span=2m per_minute(cpu_usage)");
492492
assertTrue(
493493
result.contains(
494-
"per_minute(cpu_usage)=[DIVIDE(*($1, 60000.0E0), TIMESTAMPDIFF('MILLISECOND', $0,"
495-
+ " TIMESTAMPADD('MINUTE', 2, $0)))]"));
496-
assertTrue(result.contains("per_minute(cpu_usage)=[SUM($1)]"));
494+
"per_minute(cpu_usage)=[DIVIDE(*($1, 60000.0E0), TIMESTAMPDIFF('MILLISECOND':VARCHAR,"
495+
+ " $0, TIMESTAMPADD('MINUTE':VARCHAR, 2, $0)))]"));
496+
assertTrue(result.contains("per_minute(cpu_usage)=[SUM($0)]"));
497497
}
498498

499499
@Test
500500
public void testExplainTimechartPerHour() throws IOException {
501-
var result = explainQueryYaml("source=events | timechart span=2m per_hour(cpu_usage)");
501+
var result = explainQueryToString("source=events | timechart span=2m per_hour(cpu_usage)");
502502
assertTrue(
503503
result.contains(
504-
"per_hour(cpu_usage)=[DIVIDE(*($1, 3600000.0E0), TIMESTAMPDIFF('MILLISECOND', $0,"
505-
+ " TIMESTAMPADD('MINUTE', 2, $0)))]"));
506-
assertTrue(result.contains("per_hour(cpu_usage)=[SUM($1)]"));
504+
"per_hour(cpu_usage)=[DIVIDE(*($1, 3600000.0E0), TIMESTAMPDIFF('MILLISECOND':VARCHAR,"
505+
+ " $0, TIMESTAMPADD('MINUTE':VARCHAR, 2, $0)))]"));
506+
assertTrue(result.contains("per_hour(cpu_usage)=[SUM($0)]"));
507507
}
508508

509509
@Test
510510
public void testExplainTimechartPerDay() throws IOException {
511-
var result = explainQueryYaml("source=events | timechart span=2m per_day(cpu_usage)");
511+
var result = explainQueryToString("source=events | timechart span=2m per_day(cpu_usage)");
512512
assertTrue(
513513
result.contains(
514-
"per_day(cpu_usage)=[DIVIDE(*($1, 8.64E7), TIMESTAMPDIFF('MILLISECOND', $0,"
515-
+ " TIMESTAMPADD('MINUTE', 2, $0)))]"));
516-
assertTrue(result.contains("per_day(cpu_usage)=[SUM($1)]"));
514+
"per_day(cpu_usage)=[DIVIDE(*($1, 8.64E7), TIMESTAMPDIFF('MILLISECOND':VARCHAR, $0,"
515+
+ " TIMESTAMPADD('MINUTE':VARCHAR, 2, $0)))]"));
516+
assertTrue(result.contains("per_day(cpu_usage)=[SUM($0)]"));
517517
}
518518

519519
@Test
@@ -787,7 +787,7 @@ public void testExplainRegexMatchInWhereWithScriptPushdown() throws IOException
787787
String query =
788788
String.format("source=%s | where regexp_match(name, 'hello')", TEST_INDEX_STRINGS);
789789
var result = explainQueryYaml(query);
790-
String expected = loadFromFile("expectedOutput/calcite/explain_regexp_match_in_where.yaml");
790+
String expected = loadExpectedPlan("explain_regexp_match_in_where.yaml");
791791
assertYamlEqualsIgnoreId(expected, result);
792792
}
793793

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,16 +466,15 @@ public void testBetweenWithMixedTypes() throws IOException {
466466

467467
@Test
468468
public void testBetweenWithIncompatibleTypes() throws IOException {
469-
// Plan: SAFE_CAST(NUMBER_TO_STRING(38.5:DECIMAL(3, 1))). The least restrictive type between
470-
// int, decimal, and varchar is resolved to varchar. between '35' and '38.5' is then optimized
471-
// to empty rows
469+
// With the branch's leastRestrictive() override, mixed numeric/varchar types are resolved
470+
// to a common numeric type, so the comparison works correctly: age between 35 and 38.5
472471
JSONObject actual =
473472
executeQuery(
474473
String.format(
475474
"source=%s | where age between '35' and 38.5 | fields firstname, age",
476475
TEST_INDEX_BANK));
477476
verifySchema(actual, schema("firstname", "string"), schema("age", "int"));
478-
verifyNumOfRows(actual, 0);
477+
verifyDataRows(actual, rows("Hattie", 36), rows("Elinor", 36));
479478
}
480479

481480
@Test

integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.opensearch.common.collect.MapBuilder;
2929
import org.opensearch.sql.ast.statement.ExplainMode;
3030
import org.opensearch.sql.common.setting.Settings;
31+
import org.opensearch.sql.util.MatcherUtils;
3132
import org.opensearch.sql.common.setting.Settings.Key;
3233
import org.opensearch.sql.legacy.SQLIntegTestCase;
3334
import org.opensearch.sql.protocol.response.format.Format;
@@ -420,6 +421,17 @@ protected String loadExpectedPlan(String fileName) throws IOException {
420421
} else {
421422
prefix = "expectedOutput/ppl/";
422423
}
424+
// Track the source file path for YAML regeneration
425+
if ("true".equals(System.getenv("REGENERATE_YAML"))
426+
|| Boolean.getBoolean("REGENERATE_YAML")) {
427+
// Resolve the source file path (not the build copy) using project.root
428+
String projectRoot = System.getProperty("project.root", "");
429+
if (!projectRoot.isEmpty()) {
430+
String sourcePath =
431+
projectRoot + "/src/test/resources/" + prefix + fileName;
432+
MatcherUtils.setLastExpectedFilePath(sourcePath);
433+
}
434+
}
423435
return loadFromFile(prefix + fileName);
424436
}
425437
}

integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
import com.fasterxml.jackson.databind.ObjectMapper;
2222
import com.google.common.base.Strings;
2323
import com.google.gson.JsonParser;
24+
import java.io.IOException;
2425
import java.math.BigDecimal;
26+
import java.nio.file.Files;
27+
import java.nio.file.Path;
2528
import java.util.ArrayList;
2629
import java.util.Arrays;
2730
import java.util.List;
@@ -44,6 +47,15 @@ public class MatcherUtils {
4447

4548
private static final Logger LOG = LogManager.getLogger();
4649
private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
50+
private static final ThreadLocal<String> LAST_EXPECTED_FILE_PATH = new ThreadLocal<>();
51+
52+
/**
53+
* Set the file path of the last loaded expected YAML (used for YAML regeneration). When
54+
* REGENERATE_YAML=true env var is set, the actual YAML output will be written to this path.
55+
*/
56+
public static void setLastExpectedFilePath(String path) {
57+
LAST_EXPECTED_FILE_PATH.set(path);
58+
}
4759

4860
/**
4961
* Assert field value in object by a custom matcher and getter to access the field.
@@ -406,6 +418,22 @@ public static Matcher<String> equalToIgnoreCaseAndWhiteSpace(String expectedStri
406418
* @param actual actual JSON string.
407419
*/
408420
public static void assertJsonEquals(String expected, String actual) {
421+
if ("true".equals(System.getenv("REGENERATE_YAML"))
422+
|| Boolean.getBoolean("REGENERATE_YAML")) {
423+
String filePath = LAST_EXPECTED_FILE_PATH.get();
424+
if (filePath != null
425+
&& !JsonParser.parseString(eliminatePid(expected))
426+
.equals(JsonParser.parseString(eliminatePid(actual)))) {
427+
try {
428+
Files.writeString(Path.of(filePath), actual);
429+
LOG.info("Regenerated JSON: {}", filePath);
430+
} catch (IOException e) {
431+
LOG.warn("Failed to regenerate JSON: {}", filePath, e);
432+
}
433+
}
434+
LAST_EXPECTED_FILE_PATH.remove();
435+
return;
436+
}
409437
assertEquals(
410438
JsonParser.parseString(eliminatePid(expected)),
411439
JsonParser.parseString(eliminatePid(actual)));
@@ -503,6 +531,20 @@ public static void assertYamlEqualsIgnoreId(List<String> expectedYamls, String a
503531
public static void assertYamlEquals(String expected, String actual) {
504532
String normalizedExpected = normalizeLineBreaks(expected).trim();
505533
String normalizedActual = normalizeLineBreaks(actual).trim();
534+
if ("true".equals(System.getenv("REGENERATE_YAML"))
535+
|| Boolean.getBoolean("REGENERATE_YAML")) {
536+
String filePath = LAST_EXPECTED_FILE_PATH.get();
537+
if (filePath != null && !normalizedExpected.equals(normalizedActual)) {
538+
try {
539+
Files.writeString(Path.of(filePath), actual);
540+
LOG.info("Regenerated YAML: {}", filePath);
541+
} catch (IOException e) {
542+
LOG.warn("Failed to regenerate YAML: {}", filePath, e);
543+
}
544+
}
545+
LAST_EXPECTED_FILE_PATH.remove();
546+
return; // Skip assertion when regenerating
547+
}
506548
assertEquals(
507549
formatMessage(normalizedExpected, normalizedActual), normalizedExpected, normalizedActual);
508550
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
calcite:
22
logical: |
33
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4-
LogicalProject(host=[$0], info=[GEOIP('dummy-datasource', $0)], info.dummy_sub_field=[ITEM(GEOIP('dummy-datasource', $0), 'dummy_sub_field')])
4+
LogicalProject(host=[$0], info=[GEOIP('dummy-datasource':VARCHAR, $0)], info.dummy_sub_field=[ITEM(GEOIP('dummy-datasource':VARCHAR, $0), 'dummy_sub_field')])
55
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]])
66
physical: |
7-
EnumerableCalc(expr#0=[{inputs}], expr#1=['dummy-datasource'], expr#2=[GEOIP($t1, $t0)], expr#3=['dummy_sub_field'], expr#4=[ITEM($t2, $t3)], host=[$t0], info=[$t2], info.dummy_sub_field=[$t4])
7+
EnumerableCalc(expr#0=[{inputs}], expr#1=['dummy-datasource':VARCHAR], expr#2=[GEOIP($t1, $t0)], expr#3=['dummy_sub_field'], expr#4=[ITEM($t2, $t3)], host=[$t0], info=[$t2], info.dummy_sub_field=[$t4])
88
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_weblogs]], PushDownContext=[[PROJECT->[host], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","_source":{"includes":["host"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])

integ-test/src/test/resources/expectedOutput/calcite/agg_composite_date_range_push.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ calcite:
22
logical: |
33
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
44
LogicalProject(avg(value)=[$2], span(@timestamp,1h)=[$1], value_range=[$0])
5-
LogicalAggregate(group=[{0, 1}], avg(value)=[AVG($2)])
6-
LogicalProject(value_range=[$10], span(@timestamp,1h)=[SPAN($0, 1, 'h')], value=[$2])
5+
LogicalAggregate(group=[{0, 2}], avg(value)=[AVG($1)])
6+
LogicalProject(value_range=[$10], value=[$2], span(@timestamp,1h)=[SPAN($0, 1, 'h')])
77
LogicalFilter(condition=[IS NOT NULL($0)])
88
LogicalProject(@timestamp=[$0], category=[$1], value=[$2], timestamp=[$3], _id=[$4], _index=[$5], _score=[$6], _maxscore=[$7], _sort=[$8], _routing=[$9], value_range=[CASE(<($2, 7000), 'small':VARCHAR, 'large':VARCHAR)])
99
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_time_data]])
1010
physical: |
11-
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_time_data]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0, 1},avg(value)=AVG($2)), PROJECT->[avg(value), span(@timestamp,1h), value_range], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":0,"timeout":"1m","aggregations":{"composite_buckets":{"composite":{"size":1000,"sources":[{"span(@timestamp,1h)":{"date_histogram":{"field":"@timestamp","missing_bucket":false,"order":"asc","fixed_interval":"1h"}}}]},"aggregations":{"value_range":{"range":{"field":"value","ranges":[{"key":"small","to":7000.0},{"key":"large","from":7000.0}],"keyed":true},"aggregations":{"avg(value)":{"avg":{"field":"value"}}}}}}}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])
11+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_time_data]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0, 2},avg(value)=AVG($1)), PROJECT->[avg(value), span(@timestamp,1h), value_range], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":0,"timeout":"1m","aggregations":{"composite_buckets":{"composite":{"size":1000,"sources":[{"span(@timestamp,1h)":{"date_histogram":{"field":"@timestamp","missing_bucket":false,"order":"asc","fixed_interval":"1h"}}}]},"aggregations":{"value_range":{"range":{"field":"value","ranges":[{"key":"small","to":7000.0},{"key":"large","from":7000.0}],"keyed":true},"aggregations":{"avg(value)":{"avg":{"field":"value"}}}}}}}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])

integ-test/src/test/resources/expectedOutput/calcite/agg_filter_nested.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ calcite:
22
logical: |
33
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
44
LogicalAggregate(group=[{}], george_and_jk=[COUNT($0)])
5-
LogicalProject($f0=[CASE(<($7, 'K'), 1, null:INTEGER)])
5+
LogicalProject($f1=[CASE(<($7, 'K'), 1, null:NULL)])
66
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_cascaded_nested]])
77
physical: |
88
EnumerableLimit(fetch=[10000])

0 commit comments

Comments
 (0)