Skip to content

Commit 9aad427

Browse files
ahkcsahkcs
authored
Bring CalciteWhereCommandIT to parity on the analytics-engine route (opensearch-project#5546)
* Bring CalciteWhereCommandIT to parity on the analytics-engine route Branch the tests that exercise analytics-route-incompatible behavior behind isAnalyticsParquetIndicesEnabled(): - 6 nested-field tests (in the Calcite subclass): nested fields are stripped on the parquet/composite route, which has no nested-document support. - 2 _id metadata-field tests: the analytics route doesn't expose the _id metadata field. - testDoubleEqualWithSpecialCharacters: email is dynamically mapped to a text field without a .keyword sub-field on the parquet route (the composite dataformat's dynamic mapping omits the keyword sub-field standard OpenSearch adds), so exact equality on the analyzed text field returns no rows on the DataFusion scan. firstname (explicit text+keyword) still exercises == there. Analytics-engine route: 9 failing -> 0 (32 pass, 9 documented skips). v2 route: 41/41 pass. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Centralize analytics-route skip reasons in AnalyticsRouteLimitation enum Replace per-test assumeFalse(reason, isAnalyticsParquetIndicesEnabled()) calls with assumeNotAnalytics(LIMITATION), backed by a single AnalyticsRouteLimitation enum that holds every known analytics-engine route divergence and its skip reason. This gives one greppable registry of route gaps (addressing review feedback to make the skips trackable in one place) and removes the PUT+DELETE skip reason that was copy-pasted across four IT classes. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Track CalciteWhereCommandIT AE-route skips in the gradle exclude list Add the 9 analytics-route skips from this PR to the analyticsEnabled excludeTestsMatching block in integ-test/build.gradle, following the opensearch-project#5527 / opensearch-project#5541 precedent, so the full set of tests skipped on the analytics-engine route is countable in one place. These complement the in-test assumeNotAnalytics(...) calls (which record the per-test reason via AnalyticsRouteLimitation); the gradle list is the single tracking surface for how many tests the route skips. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 87c0a13 commit 9aad427

9 files changed

Lines changed: 127 additions & 44 deletions

File tree

‎integ-test/build.gradle‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,26 @@ task integTestRemote(type: RestIntegTestTask) {
10821082
// CalciteAliasFieldAggregationIT: raw-PUT alias index can't be created on the AE route
10831083
// and every test queries alias fields directly — whole class doomed.
10841084
excludeTestsMatching 'org.opensearch.sql.calcite.remote.CalciteAliasFieldAggregationIT'
1085+
1086+
// === Excludes: WhereCommandIT / CalciteWhereCommandIT route divergences (#5546) ===
1087+
// Each test also carries an in-test assumeNotAnalytics(...) recording the reason (see
1088+
// AnalyticsRouteLimitation); listed here so the AE-route skip set stays countable in one
1089+
// place. Reasons:
1090+
// - Nested-field queries: the parquet/composite store has no nested-document support, so
1091+
// nested fields are stripped at load (#5541). Defined only in CalciteWhereCommandIT.
1092+
excludeTestsMatching '*CalciteWhereCommandIT.testFilterOnComputedNestedFields'
1093+
excludeTestsMatching '*CalciteWhereCommandIT.testFilterOnNestedAndRootFields'
1094+
excludeTestsMatching '*CalciteWhereCommandIT.testFilterOnNestedFields'
1095+
excludeTestsMatching '*CalciteWhereCommandIT.testFilterOnMultipleCascadedNestedFields'
1096+
excludeTestsMatching '*CalciteWhereCommandIT.testScriptFilterOnDifferentNestedHierarchyShouldThrow'
1097+
excludeTestsMatching '*CalciteWhereCommandIT.testAggFilterOnNestedFields'
1098+
// - _id metadata field not exposed on parquet-backed scans. Defined in the base
1099+
// WhereCommandIT and inherited by CalciteWhereCommandIT; the glob matches both runs.
1100+
excludeTestsMatching '*WhereCommandIT.testWhereWithMetadataFields'
1101+
excludeTestsMatching '*WhereCommandIT.testWhereWithMetadataFields2'
1102+
// - Exact == on a dynamically-mapped string (email) returns no rows: dynamic mapping
1103+
// omits the .keyword sub-field on the AE route.
1104+
excludeTestsMatching '*WhereCommandIT.testDoubleEqualWithSpecialCharacters'
10851105
}
10861106
}
10871107

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66
package org.opensearch.sql.calcite.remote;
77

8-
import static org.junit.Assume.assumeFalse;
98
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ARRAY;
109
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_SIMPLE;
10+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.DOC_MUTATION;
1111
import static org.opensearch.sql.util.MatcherUtils.rows;
1212
import static org.opensearch.sql.util.MatcherUtils.schema;
1313
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -296,10 +296,7 @@ public void testExpandWithEval() throws Exception {
296296

297297
@Test
298298
public void testExpandEmptyArray() throws Exception {
299-
assumeFalse(
300-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
301-
+ " (analytics-engine storage path) does not support.",
302-
isAnalyticsParquetIndicesEnabled());
299+
assumeNotAnalytics(DOC_MUTATION);
303300
final int docId = 6;
304301
Request insertRequest =
305302
new Request(
@@ -329,10 +326,7 @@ public void testExpandEmptyArray() throws Exception {
329326

330327
@Test
331328
public void testExpandOnNullField() throws Exception {
332-
assumeFalse(
333-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
334-
+ " (analytics-engine storage path) does not support.",
335-
isAnalyticsParquetIndicesEnabled());
329+
assumeNotAnalytics(DOC_MUTATION);
336330
final int docId = 6;
337331
Request insertRequest =
338332
new Request(

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
package org.opensearch.sql.calcite.remote;
77

8-
import static org.junit.Assume.assumeFalse;
98
import static org.opensearch.sql.legacy.TestsConstants.*;
9+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.DOC_MUTATION;
1010
import static org.opensearch.sql.util.MatcherUtils.*;
1111

1212
import java.io.IOException;
@@ -508,10 +508,7 @@ public void testStreamstatsCurrentAndWindowWithNull() throws IOException {
508508

509509
@Test
510510
public void testStreamstatsGlobal() throws IOException {
511-
assumeFalse(
512-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
513-
+ " (analytics-engine storage path) does not support.",
514-
isAnalyticsParquetIndicesEnabled());
511+
assumeNotAnalytics(DOC_MUTATION);
515512
final int docId = 5;
516513
Request insertRequest =
517514
new Request(
@@ -671,10 +668,7 @@ public void testStreamstatsGlobalWithNullBucket() throws IOException {
671668

672669
@Test
673670
public void testStreamstatsReset() throws IOException {
674-
assumeFalse(
675-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
676-
+ " (analytics-engine storage path) does not support.",
677-
isAnalyticsParquetIndicesEnabled());
671+
assumeNotAnalytics(DOC_MUTATION);
678672
final int docId = 5;
679673
Request insertRequest =
680674
new Request(
@@ -943,10 +937,7 @@ public void testMultipleStreamstatsWithNull1() throws IOException {
943937

944938
@Test
945939
public void testMultipleStreamstatsWithNull2() throws IOException {
946-
assumeFalse(
947-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
948-
+ " (analytics-engine storage path) does not support.",
949-
isAnalyticsParquetIndicesEnabled());
940+
assumeNotAnalytics(DOC_MUTATION);
950941
final int docId = 5;
951942
Request insertRequest =
952943
new Request(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_CASCADED_NESTED;
99
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DEEP_NESTED;
1010
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_SIMPLE;
11+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.NESTED_FIELDS;
1112
import static org.opensearch.sql.util.MatcherUtils.rows;
1213
import static org.opensearch.sql.util.MatcherUtils.schema;
1314
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -23,6 +24,7 @@
2324
import org.opensearch.sql.ppl.WhereCommandIT;
2425

2526
public class CalciteWhereCommandIT extends WhereCommandIT {
27+
2628
@Override
2729
public void init() throws Exception {
2830
super.init();
@@ -40,6 +42,7 @@ protected String getIncompatibleTypeErrMsg() {
4042

4143
@Test
4244
public void testFilterOnComputedNestedFields() throws IOException {
45+
assumeNotAnalytics(NESTED_FIELDS);
4346
JSONObject result =
4447
executeQuery(
4548
String.format(
@@ -52,6 +55,7 @@ public void testFilterOnComputedNestedFields() throws IOException {
5255

5356
@Test
5457
public void testFilterOnNestedAndRootFields() throws IOException {
58+
assumeNotAnalytics(NESTED_FIELDS);
5559
JSONObject result =
5660
executeQuery(
5761
String.format(
@@ -64,6 +68,7 @@ public void testFilterOnNestedAndRootFields() throws IOException {
6468

6569
@Test
6670
public void testFilterOnNestedFields() throws IOException {
71+
assumeNotAnalytics(NESTED_FIELDS);
6772
// address is a nested object
6873
JSONObject result1 =
6974
executeQuery(
@@ -83,6 +88,7 @@ public void testFilterOnNestedFields() throws IOException {
8388

8489
@Test
8590
public void testFilterOnMultipleCascadedNestedFields() throws IOException {
91+
assumeNotAnalytics(NESTED_FIELDS);
8692
// SQL's static type system does not allow returning list[int] in place of int
8793
enabledOnlyWhenPushdownIsEnabled();
8894
JSONObject result =
@@ -126,6 +132,7 @@ public void testFilterOnMultipleCascadedNestedFields() throws IOException {
126132

127133
@Test
128134
public void testScriptFilterOnDifferentNestedHierarchyShouldThrow() throws IOException {
135+
assumeNotAnalytics(NESTED_FIELDS);
129136
enabledOnlyWhenPushdownIsEnabled();
130137
Throwable t =
131138
assertThrows(
@@ -144,6 +151,7 @@ public void testScriptFilterOnDifferentNestedHierarchyShouldThrow() throws IOExc
144151

145152
@Test
146153
public void testAggFilterOnNestedFields() throws IOException {
154+
assumeNotAnalytics(NESTED_FIELDS);
147155
enabledOnlyWhenPushdownIsEnabled();
148156
JSONObject result =
149157
executeQuery(

‎integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66
package org.opensearch.sql.ppl;
77

8-
import static org.junit.Assume.assumeFalse;
98
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK;
109
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE;
10+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.DOC_MUTATION;
1111
import static org.opensearch.sql.util.MatcherUtils.rows;
1212
import static org.opensearch.sql.util.MatcherUtils.schema;
1313
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -1566,10 +1566,7 @@ public void testExtract() throws IOException {
15661566

15671567
@Test
15681568
public void testCompareAgainstUTCDate() throws IOException {
1569-
assumeFalse(
1570-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
1571-
+ " (analytics-engine storage path) does not support.",
1572-
isAnalyticsParquetIndicesEnabled());
1569+
assumeNotAnalytics(DOC_MUTATION);
15731570
LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC);
15741571
String isoTimestamp = now.format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'"));
15751572
String pplTimestamp = now.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"));

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.opensearch.sql.legacy.SQLIntegTestCase;
3939
import org.opensearch.sql.legacy.TestUtils;
4040
import org.opensearch.sql.protocol.response.format.Format;
41+
import org.opensearch.sql.util.AnalyticsRouteLimitation;
4142
import org.opensearch.sql.util.RetryProcessor;
4243

4344
/** OpenSearch Rest integration test base for PPL testing. */
@@ -69,6 +70,16 @@ public static boolean isAnalyticsParquetIndicesEnabled() {
6970
return TestUtils.AnalyticsIndexConfig.isEnabled();
7071
}
7172

73+
/**
74+
* Skip the current test on the analytics-engine route because of a known limitation of that route
75+
* (no-op on the v2/Calcite path). The {@link AnalyticsRouteLimitation} registry holds the reason,
76+
* so a skip reads as {@code assumeNotAnalytics(NESTED_FIELDS)} and every route gap stays
77+
* greppable in one place.
78+
*/
79+
public static void assumeNotAnalytics(AnalyticsRouteLimitation limitation) {
80+
Assume.assumeFalse(limitation.reason(), isAnalyticsParquetIndicesEnabled());
81+
}
82+
7283
protected JSONObject executeQuery(String query) throws IOException {
7384
return jsonify(executeQueryToString(query));
7485
}

‎integ-test/src/test/java/org/opensearch/sql/ppl/SearchCommandIT.java‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
package org.opensearch.sql.ppl;
77

8-
import static org.junit.Assume.assumeFalse;
98
import static org.opensearch.sql.legacy.TestsConstants.*;
9+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.DOC_MUTATION;
1010
import static org.opensearch.sql.util.MatcherUtils.columnName;
1111
import static org.opensearch.sql.util.MatcherUtils.rows;
1212
import static org.opensearch.sql.util.MatcherUtils.schema;
@@ -1054,10 +1054,7 @@ public void testSearchWithNumericTimeRange() throws IOException {
10541054

10551055
@Test
10561056
public void testSearchTimeModifierWithSnappedWeek() throws IOException {
1057-
assumeFalse(
1058-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
1059-
+ " (analytics-engine storage path) does not support.",
1060-
isAnalyticsParquetIndicesEnabled());
1057+
assumeNotAnalytics(DOC_MUTATION);
10611058
// Test whether alignment to weekday works
10621059

10631060
final int docId = 101;
@@ -1146,10 +1143,7 @@ public void testSearchTimeModifierWithSnappedWeek() throws IOException {
11461143

11471144
@Test
11481145
public void testSearchWithRelativeTimeModifiers() throws IOException {
1149-
assumeFalse(
1150-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
1151-
+ " (analytics-engine storage path) does not support.",
1152-
isAnalyticsParquetIndicesEnabled());
1146+
assumeNotAnalytics(DOC_MUTATION);
11531147
final int docId = 101;
11541148

11551149
LocalDateTime currentTime = LocalDateTime.now(ZoneOffset.UTC);
@@ -1198,10 +1192,7 @@ public void testSearchWithRelativeTimeModifiers() throws IOException {
11981192

11991193
@Test
12001194
public void testSearchWithTimeUnitSnapping() throws IOException {
1201-
assumeFalse(
1202-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
1203-
+ " (analytics-engine storage path) does not support.",
1204-
isAnalyticsParquetIndicesEnabled());
1195+
assumeNotAnalytics(DOC_MUTATION);
12051196
final int docId = 101;
12061197

12071198
LocalDateTime currentHour = LocalDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.HOURS);
@@ -1250,10 +1241,7 @@ public void testSearchWithTimeUnitSnapping() throws IOException {
12501241

12511242
@Test
12521243
public void testSearchWithQuarterlyModifiers() throws IOException {
1253-
assumeFalse(
1254-
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine"
1255-
+ " (analytics-engine storage path) does not support.",
1256-
isAnalyticsParquetIndicesEnabled());
1244+
assumeNotAnalytics(DOC_MUTATION);
12571245
final int docId = 101;
12581246

12591247
LocalDateTime currentQuarter =

‎integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT;
1010
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES;
1111
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME;
12+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.DYNAMIC_STRING_NO_KEYWORD;
13+
import static org.opensearch.sql.util.AnalyticsRouteLimitation.ID_METADATA;
1214
import static org.opensearch.sql.util.MatcherUtils.rows;
1315
import static org.opensearch.sql.util.MatcherUtils.schema;
1416
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -216,6 +218,7 @@ public void testIsNotNullPredicate() throws IOException {
216218

217219
@Test
218220
public void testWhereWithMetadataFields() throws IOException {
221+
assumeNotAnalytics(ID_METADATA);
219222
JSONObject result =
220223
executeQuery(
221224
String.format("source=%s | where _id='1' | fields firstname", TEST_INDEX_ACCOUNT));
@@ -224,6 +227,7 @@ public void testWhereWithMetadataFields() throws IOException {
224227

225228
@Test
226229
public void testWhereWithMetadataFields2() throws IOException {
230+
assumeNotAnalytics(ID_METADATA);
227231
JSONObject result =
228232
executeQuery(String.format("source=%s | where _id='1'", TEST_INDEX_ACCOUNT));
229233
verifyDataRows(
@@ -531,6 +535,7 @@ public void testDoubleEqualCaseSensitiveStringComparison() throws IOException {
531535

532536
@Test
533537
public void testDoubleEqualWithSpecialCharacters() throws IOException {
538+
assumeNotAnalytics(DYNAMIC_STRING_NO_KEYWORD);
534539
// Test == with strings containing special characters
535540
JSONObject result =
536541
executeQuery(

‎integ-test/src/test/java/org/opensearch/sql/util/AnalyticsRouteLimitation.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.util;
7+
8+
/**
9+
* Single registry of the known behavioral divergences of the analytics-engine route
10+
* (parquet/composite store + DataFusion backend, activated by {@code
11+
* -Dtests.analytics.parquet_indices=true}). Each constant carries the reason a test is skipped on
12+
* that route.
13+
*
14+
* <p>Pair it with {@code PPLIntegTestCase.assumeNotAnalytics(...)} so a skip reads as {@code
15+
* assumeNotAnalytics(NESTED_FIELDS)} instead of a copy-pasted string literal. Keeping every reason
16+
* here makes the full set of analytics-route gaps greppable in one place — both for humans tracking
17+
* what still needs fixing and as a single block of context to hand an agent for bulk triage.
18+
*/
19+
public enum AnalyticsRouteLimitation {
20+
/**
21+
* The parquet/composite store has no nested-document support, so nested fields are stripped from
22+
* the dataset at load (#5541) and queries that reference them resolve against fields that don't
23+
* exist on this route.
24+
*/
25+
NESTED_FIELDS(
26+
"Nested-field queries can't run on the analytics-engine route: the parquet/composite store"
27+
+ " has no nested-document support, so nested fields are stripped from the dataset at"
28+
+ " load (#5541)."),
29+
30+
/**
31+
* Parquet-backed scans surface only mapped document fields, so the {@code _id} metadata field is
32+
* not exposed on this route.
33+
*/
34+
ID_METADATA(
35+
"The analytics-engine route doesn't expose the _id metadata field (parquet-backed scans"
36+
+ " surface only mapped document fields)."),
37+
38+
/**
39+
* The composite dataformat's dynamic mapping gives string fields a {@code text} mapping without
40+
* the {@code .keyword} sub-field that standard OpenSearch adds, so exact equality ({@code =} /
41+
* {@code ==}) on a dynamically-mapped string silently returns no rows on the DataFusion scan.
42+
* Explicitly mapped {@code text}+{@code keyword} fields are unaffected.
43+
*/
44+
DYNAMIC_STRING_NO_KEYWORD(
45+
"Exact equality (= / ==) on a dynamically-mapped string field returns no rows on the"
46+
+ " analytics-engine route: the composite dataformat's dynamic mapping omits the .keyword"
47+
+ " sub-field that standard OpenSearch adds, and the DataFusion scan can't match on an"
48+
+ " analyzed text field."),
49+
50+
/**
51+
* The analytics-engine storage path ({@code DataFormatAwareEngine}) does not support in-place
52+
* document mutation, so tests that seed state via raw {@code PUT}+{@code DELETE} can't run on
53+
* this route.
54+
*/
55+
DOC_MUTATION(
56+
"Test mutates docs via PUT+DELETE, which DataFormatAwareEngine (analytics-engine storage"
57+
+ " path) does not support.");
58+
59+
private final String reason;
60+
61+
AnalyticsRouteLimitation(String reason) {
62+
this.reason = reason;
63+
}
64+
65+
/** Human-readable explanation surfaced as the JUnit skip message. */
66+
public String reason() {
67+
return reason;
68+
}
69+
}

0 commit comments

Comments
 (0)