Stabilize more PPL ITs on the analytics-engine route (sort/streamstats/IP-UDT/metadata/strip-verifier)#5566
Merged
ahkcs merged 1 commit intoJun 19, 2026
Conversation
a225c00 to
c4583ab
Compare
…s/IP-UDT/metadata/strip-verifier) Brings 16 more PPL IT classes to parity on the analytics-engine route (-Dtests.analytics.parquet_indices=true). Route-only divergences are gated AE-only via @RequiresCapability + a matching build.gradle excludeTestsMatching; the v2/Calcite path runs every test unchanged. Strip-verifier (the opensearch-project#5541 guardrail): - AnalyticsUnsupportedFieldStripVerifyIT was failing because 8 datasets carry a multi-value JSON array for a scalar-mapped field, which the parquet store rejects at bulk load. That's a cardinality limitation, not an unsupported field *type*, so it's out of scope for the type strip — the same situation as the existing `join` out-of-scope skip. Added a curated MULTI_VALUE_DATASETS allowlist + safeToSkipForMultiValueLoad that skips only the exact multi-value signature on a known dataset; any other failure still surfaces loudly, and Legs 2-3 still type-check every index that loads. Init-load contamination (not divergences — fixed, not gated): - CalciteWhereCommandIT failed on testDoubleEqual* because init() loaded game_of_thrones (base) and deep_nested (subclass), both multi-value datasets whose bulk-load failure aborted init() and mislabeled the first test. Guarded both loads with isAnalyticsParquetIndicesEnabled(); no test in the hierarchy queries them on the AE route. 32/32 now pass. Non-deterministic sort ties stabilized (not gated — full coverage kept): - The three CalcitePPLSortIT tie tests (testSortWithNullValue, testSortAgeAndFieldsNameAge, testSortWithAutoCast) sorted on a non-unique key, so the tied rows had no defined order and the AE route ordered them differently than the captured Lucene doc order. Added a unique secondary sort key (firstname) to each, making the order deterministic and engine-independent. Verified identical on BOTH routes: 18/18 on AE and 18/18 on v2/Calcite. No gate needed. Engine divergences gated (new capabilities): - INVALID_DATETIME_ERROR_SHAPE: dayname/monthname over an invalid literal throw a different message shape (2 tests) - RAND_SEED_UNSUPPORTED: seeded RAND(seed) is rejected on AE - IP_UDT_BINARY_REPRESENTATION: the IP UDT is materialized as BINARY, so cast(... as IP) and cidrmatch over an IP column fail (2 tests) - TIME_TYPE_WIDENED_TO_TIMESTAMP: a TIME field reads back as TIMESTAMP, defeating TIMEDIFF's [TIME,TIME] signature - BINARY_FIELD_STRIPPED: binary fields are stripped at load - VALUES_LIMIT_NOT_HONORED: values()/list() ignore the configured limit - INDEX_METADATA: _index metadata not exposed (sibling of ID_METADATA) - CROSS_INDEX_OBJECT_LEAF_MERGE: an object leaf in only some wildcard member indices resolves to FIELD_NOT_FOUND - TEXT_KEYWORD_PUSHDOWN_REWRITE: like() doesn't rewrite to .keyword in the explain plan (no Lucene term-pushdown) - LUCENE_PUSHDOWN_EXPLAIN: a test asserting a Lucene SORT-> pushdown fragment can't match the DataFusion plan Reused existing capabilities: - WILDCARD_COLUMN_ORDER: streamstats carries all source columns through; AE returns them in a different column order (values and row order are correct, so a sort can't fix it) (4 CalciteReverseCommandIT tests) - HEAD_WITHOUT_STABLE_SORT: the non-determinism is which rows head N keeps, before the trailing sort, so a sort can't recover it (testHeadThenSort, testAppendWithMergedColumn) - DEDUP_NONDETERMINISTIC: consecutive dedup has no working V2 fallback on the AE route Out of scope: - FieldsCommandIT.testEnhancedFieldsWhenCalciteDisabled asserts the Calcite-DISABLED error; the AE route is always Calcite-enabled. build.gradle exclude only. Results (this batch, on the AE route): 16 classes, 0 failures (was 24 failures), with the 3 sort tests now passing rather than skipped. V2 baseline: 0 failures, only pre-existing/by-design skips (none from these gates). Signed-off-by: Kai Huang <ahkcs@amazon.com>
c4583ab to
539cef4
Compare
mengweieric
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Brings 16 more PPL integration test classes to parity on the analytics-engine route (
-Dtests.analytics.parquet_indices=true, which routes PPL through the parquet/composite store to DataFusion). Route-only divergences are skipped only when the analytics flag is on, via@RequiresCapability+ a matchingbuild.gradleexcludeTestsMatching; the v2/Calcite path runs every test unchanged. Continues #5560 / #5561 / #5562 / #5564.A key finding: several of the originally-reported "failures" were init-load contamination — a class loading a multi-value dataset (
game_of_thrones,deep_nested, …) ininit()throws at bulk load on the AE route, abortinginit()and mislabeling whichever test ran first. Those are fixed (guarded loads), not gated.Pass rate (this batch, on the analytics-engine route)
AE route: 16 classes, 373 run, 0 failures (was 24 failures).
V2 baseline: 408 run, 0 failures, 2 pre-existing/by-design skips (none from these gates).
Strip-verifier (the #5541 guardrail)
AnalyticsUnsupportedFieldStripVerifyITwas failing because 8 datasets carry a multi-value JSON array for a scalar-mapped field, which the parquet store rejects at bulk load. That's a cardinality limitation, not an unsupported type, so it's out of scope for the type strip — the same situation as the existingjoinout-of-scope skip. Added a curatedMULTI_VALUE_DATASETSallowlist +safeToSkipForMultiValueLoadthat skips only the exact multi-value signature on a known dataset; any other failure still surfaces loudly, and Legs 2-3 still type-check every index that loads.Init-load contamination (fixed, not gated)
CalciteWhereCommandITfailed ontestDoubleEqual*becauseinit()loadedgame_of_thrones(base) anddeep_nested(subclass), both multi-value datasets whose bulk-load failure abortedinit(). Guarded both withisAnalyticsParquetIndicesEnabled(); no test in the hierarchy queries them. 32/32 now pass.Not divergences — handled without skipping result assertions
Where a test branched on
isCalciteEnabled()for a type, the AE route runs the Calcite path but the cluster setting reads false — those were already addressed in #5564. This batch contains no test-expectation edits on dual-route classes.New capabilities
SORT_TIE_ORDER_UNSTABLE,INVALID_DATETIME_ERROR_SHAPE,RAND_SEED_UNSUPPORTED,IP_UDT_BINARY_REPRESENTATION,TIME_TYPE_WIDENED_TO_TIMESTAMP,BINARY_FIELD_STRIPPED,VALUES_LIMIT_NOT_HONORED,INDEX_METADATA,CROSS_INDEX_OBJECT_LEAF_MERGE,TEXT_KEYWORD_PUSHDOWN_REWRITE,LUCENE_PUSHDOWN_EXPLAIN.Reused capabilities
WILDCARD_COLUMN_ORDER— streamstats carries all source columns through; AE returns them in a different order (4 tests)HEAD_WITHOUT_STABLE_SORT—head Nwithout a stable sort (testHeadThenSort,testAppendWithMergedColumn)DEDUP_NONDETERMINISTIC— consecutive dedup has no working V2 fallback on the AE routeOut of scope
FieldsCommandIT.testEnhancedFieldsWhenCalciteDisabledasserts the Calcite-disabled error, but the AE route is always Calcite-enabled —build.gradleexclude only.Worth a real fix later (gated for now, flagged in capability docs)
IP_UDT_BINARY_REPRESENTATION— the IP UDT is materialized asBINARYon the route (next UDT-on-parquet gap after the DATE/TIME UDT fix).VALUES_LIMIT_NOT_HONORED—PplAggregateCallRewriteremits no LIMIT forvalues()/list().Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.