From f19eeda35c30536f11322f07296e84d148c46a6f Mon Sep 17 00:00:00 2001 From: Kai Huang Date: Mon, 15 Jun 2026 15:20:23 -0700 Subject: [PATCH] Bring CalcitePPLEnhancedCoalesceIT to parity on the analytics-engine route Four ordering-sensitive tests failed on the analytics route only because they used 'head N' without a sort, and the analytics scan surfaces raw-PUT docs ahead of bulk-loaded docs (non-deterministic row set). Fixed in place by adding an explicit 'sort - age' where the sort key is unique over the head window (or the tie projects identical values), so the expectations are unchanged and hold on both routes: - testCoalesceNested - testCoalesceEmptyFieldWithFallback - testCoalesceWithMultipleNonExistentFields - testCoalesceWithNullLiteralAndIntegerField Three tests are skipped on the analytics route (assumeNotAnalytics + gradle exclude list, recorded as AnalyticsRouteLimitation constants): - testCoalesceBasic, testCoalesceWithMixedTypes: 'head 3' whose third row needs a nullable tiebreak (age=25 tie between a real doc and a raw-PUT null-name doc); null placement diverges between the v2/Calcite and analytics routes, so a sort can't make the expectation hold on both. - testCoalesceWithAllNonExistentFields: COALESCE over all-untyped-null operands is rejected by the analytics-engine capability registry (No backend supports scalar function [COALESCE]); the v2 path returns an undefined-typed null (#5175). Genuine engine gap. The v2/Calcite path is unchanged: all 19 tests run and pass there. Signed-off-by: Kai Huang --- integ-test/build.gradle | 11 ++++++++ .../remote/CalcitePPLEnhancedCoalesceIT.java | 19 ++++++++------ .../sql/util/AnalyticsRouteLimitation.java | 25 ++++++++++++++++++- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 0d46f7feea..e744365a15 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -1124,6 +1124,17 @@ task integTestRemote(type: RestIntegTestTask) { excludeTestsMatching '*CalciteBinCommandIT.testStatsWithBinsOnTimeField_Avg' excludeTestsMatching '*CalciteBinCommandIT.testStatsWithBinsOnTimeAndTermField_Count' excludeTestsMatching '*CalciteBinCommandIT.testStatsWithBinsOnTimeAndTermField_Avg' + + // === Excludes: CalcitePPLEnhancedCoalesceIT route divergences === + // Each test also carries an in-test assumeNotAnalytics(...) recording the reason (see + // AnalyticsRouteLimitation). The other ordering-sensitive coalesce tests were fixed in + // place with an explicit `sort` instead of being skipped. + // - COALESCE over all-untyped-null operands is rejected by the capability registry. + excludeTestsMatching '*CalcitePPLEnhancedCoalesceIT.testCoalesceWithAllNonExistentFields' + // - head N without a sort unique over the head window is non-deterministic, and the + // nullable tiebreak these would need orders nulls differently than v2/Calcite. + excludeTestsMatching '*CalcitePPLEnhancedCoalesceIT.testCoalesceWithMixedTypes' + excludeTestsMatching '*CalcitePPLEnhancedCoalesceIT.testCoalesceBasic' } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLEnhancedCoalesceIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLEnhancedCoalesceIT.java index a0e6c2679f..40974ddce3 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLEnhancedCoalesceIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLEnhancedCoalesceIT.java @@ -6,6 +6,8 @@ package org.opensearch.sql.calcite.remote; import static org.opensearch.sql.legacy.TestsConstants.*; +import static org.opensearch.sql.util.AnalyticsRouteLimitation.COALESCE_ALL_NULL_OPERANDS; +import static org.opensearch.sql.util.AnalyticsRouteLimitation.HEAD_WITHOUT_STABLE_SORT; import static org.opensearch.sql.util.MatcherUtils.*; import java.io.IOException; @@ -37,7 +39,7 @@ public void init() throws Exception { @Test public void testCoalesceBasic() throws IOException { - + assumeNotAnalytics(HEAD_WITHOUT_STABLE_SORT); JSONObject actual = executeQuery( String.format( @@ -53,7 +55,7 @@ public void testCoalesceBasic() throws IOException { @Test public void testCoalesceWithMixedTypes() throws IOException { - + assumeNotAnalytics(HEAD_WITHOUT_STABLE_SORT); JSONObject actual = executeQuery( String.format( @@ -121,7 +123,7 @@ public void testCoalesceNested() throws IOException { executeQuery( String.format( "source=%s | eval result1 = coalesce(name, 'default'), result2 = coalesce(result1," - + " age) | fields name, age, result1, result2 | head 2", + + " age) | sort - age | fields name, age, result1, result2 | head 2", TEST_INDEX_STATE_COUNTRY_WITH_NULL)); verifySchema( @@ -153,8 +155,8 @@ public void testCoalesceWithMultipleNonExistentFields() throws IOException { JSONObject actual = executeQuery( String.format( - "source=%s | eval result = coalesce(field1, field2, name, 'fallback') | fields" - + " name, result | head 1", + "source=%s | eval result = coalesce(field1, field2, name, 'fallback') | sort - age" + + " | fields name, result | head 1", TEST_INDEX_STATE_COUNTRY_WITH_NULL)); verifySchema(actual, schema("name", "string"), schema("result", "string")); @@ -163,7 +165,7 @@ public void testCoalesceWithMultipleNonExistentFields() throws IOException { @Test public void testCoalesceWithAllNonExistentFields() throws IOException { - + assumeNotAnalytics(COALESCE_ALL_NULL_OPERANDS); JSONObject actual = executeQuery( String.format( @@ -221,7 +223,8 @@ public void testCoalesceWithNullLiteralAndIntegerField() throws IOException { JSONObject actual = executeQuery( String.format( - "source=%s | eval result = coalesce(null, age) | fields age, result | head 3", + "source=%s | eval result = coalesce(null, age) | sort - age | fields age, result |" + + " head 3", TEST_INDEX_STATE_COUNTRY_WITH_NULL)); verifySchema(actual, schema("age", "int"), schema("result", "int")); @@ -263,7 +266,7 @@ public void testCoalesceEmptyFieldWithFallback() throws IOException { executeQuery( String.format( "source=%s | eval empty_field = '' | eval result = coalesce(empty_field, name) |" - + " fields name, result | head 1", + + " sort - age | fields name, result | head 1", TEST_INDEX_STATE_COUNTRY_WITH_NULL)); verifySchema(actual, schema("name", "string"), schema("result", "string")); diff --git a/integ-test/src/test/java/org/opensearch/sql/util/AnalyticsRouteLimitation.java b/integ-test/src/test/java/org/opensearch/sql/util/AnalyticsRouteLimitation.java index b16c8a1065..41d718d84d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/AnalyticsRouteLimitation.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/AnalyticsRouteLimitation.java @@ -85,7 +85,30 @@ public enum AnalyticsRouteLimitation { BIN_TIME_FIELD_BUCKETING( "bin on a time field then grouping by it diverges on the analytics-engine route: the bucket" + " column is typed string (not timestamp) and the bucket set differs from the v2/Calcite" - + " path."); + + " path."), + + /** + * {@code COALESCE} over operands that are all untyped NULL (e.g. {@code coalesce(field1, field2, + * field3)} where every field is missing from the mapping) is rejected by the analytics-engine + * capability registry with {@code No backend supports scalar function [COALESCE] among + * [datafusion]}. The v2/Calcite path returns an {@code undefined}-typed null instead (#5175). + */ + COALESCE_ALL_NULL_OPERANDS( + "COALESCE over all-untyped-null operands is unsupported on the analytics-engine route (the" + + " capability registry rejects it); the v2/Calcite path returns an undefined-typed" + + " null."), + + /** + * A {@code head N} without a stable {@code sort} returns a non-deterministic row set on the + * analytics-engine route — raw-{@code PUT} docs land in a separate segment and can sort ahead of + * the bulk-loaded docs. Adding a {@code sort} fixes it only when the sort key is unique over the + * head window; when ties fall back to a nullable key, null placement diverges between the + * v2/Calcite and analytics routes, so the row set still differs. + */ + HEAD_WITHOUT_STABLE_SORT( + "head N without a sort on a key that is unique over the head window is non-deterministic on" + + " the analytics-engine route, and a nullable tiebreak orders nulls differently than the" + + " v2/Calcite path."); private final String reason;