[QA] Add UnionCommandIT for analytics-engine route

[ahkcs]

Description

Adds a self-contained UnionCommandIT under sandbox/qa/analytics-engine-rest, mirroring the union surface from CalciteUnionCommandIT in opensearch-project/sql so the analytics-engine path can be verified inside core without cross-plugin dependencies on the SQL plugin.

Scope change after rebase

This PR was originally a two-commit change: (1) a Java-side Timestamp → Timestamp coercion patch in DatafusionReduceSink.coerceToDeclaredSchema, plus (2) the new IT. PR #21690 (merged 2026-05-16) moved schema coercion handling from Java to Rust — DatafusionReduceSink.coerceToDeclaredSchema was deleted in favour of a typesMatch tripwire that treats any two Timestamp types as matching, and the native side picks up the lowered precision from substrait at registration time via derive_schema_from_partial_plan.

The Java-side commit is therefore obsolete and has been dropped on rebase. Only the IT remains.

Validation

UnionCommandIT is run against upstream/main (post-#21690) with no Java-side coercion patch:

testBasicUnionTwoSubsearches              ✅
testUnionThreeSubsearches                 ✅
testUnionMidPipelineSingleExplicitDataset ✅
testUnionPreservesDuplicates              ✅
testUnionWithEmptySubsearch               ✅
testUnionWithAllEmptyDatasets             ✅
testUnionWithSingleSubsearchThrowsError   ✅

Test class	Before	After
UnionCommandIT (new)	n/a	7 / 7
DatafusionReduceSinkTests (existing, post-#21690)	8 / 8	8 / 8

This validates @sandeshkr419's claim that the Java-side patch is no longer needed: the union surface, including the Timestamp cross-precision scenario that originally motivated the patch, works end-to-end on the Rust-side coercion alone.

What the IT covers

Seven tests against POST /_analytics/ppl on the calcs dataset:

• testBasicUnionTwoSubsearches — two-subsearch shape
• testUnionThreeSubsearches — three-subsearch shape
• testUnionMidPipelineSingleExplicitDataset — search ... | union [search ...]
• testUnionPreservesDuplicates — three identical branches → 3× row count
• testUnionWithEmptySubsearch — one branch returns 0 rows
• testUnionWithAllEmptyDatasets — both branches empty
• testUnionWithSingleSubsearchThrowsError — validation error surfaces

Assertions are dataset-independent (sums of branch counts, not hardcoded totals) so the suite stays resilient to calcs bulk-data changes.

calcs carries date / time / datetime columns, so each branch's partial output schema includes Timestamp types — the reduce sink's loose-match tripwire on Timestamp is exercised on every test here even when the final projection is | stats count(), making this IT the end-to-end regression net for the Timestamp handling that #21690 introduced.

[sandeshkr419]

I had moved the schema coercion handling on java side entirely: #21690 and moved it to rust to eliminate the need of this - I think your change should not be needed now. Can you please validate this with your tests.

Can you please still add up the test-cases for these cases just so that we can ensure cases covered by you are part of tests.

[ahkcs]

Confirmed — after rebasing on main (which now includes #21690), the Java-side coerceToDeclaredSchema is gone and the union surface works end-to-end without my Timestamp patch. The Java commit is dropped on rebase.

Validation on upstream/main + this branch (no Java-side coercion):

• UnionCommandIT (this PR): 7 / 7 ✅
• DatafusionReduceSinkTests (post-[Analytics Engine] Eliminate string/utf8 views & timestamp field coercion across the data-node→coordinator pipeline #21690): 8 / 8 ✅

calcs has date / time / datetime columns, so each branch's partial output schema carries Timestamp types through to the reduce sink's typesMatch tripwire. That means the IT does exercise the Timestamp → Timestamp loose-match path on every test even when the final projection is | stats count() — so the case my unit tests used to cover is now covered end-to-end here. The original four unit-pair coercion paths (SEC/MILLI/MICRO/NANO rescaling, overflow, timezone metadata) are now Rust-side concerns under derive_schema_from_partial_plan and not directly callable from Java, so I haven't re-added equivalents to the Java test surface — happy to add Rust-side coverage if you'd like, just point me at the right test module.

PR title / description updated to reflect the reduced scope.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 42caf0f)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 42caf0f

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Ensure dataset provisioned before test The test doesn't call ensureCalcsProvisioned() before executing the query, which means the calcs dataset may not exist when the test runs. This could cause the test to fail for the wrong reason or produce inconsistent results. sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/UnionCommandIT.java [129-134] -public void testUnionWithSingleSubsearchThrowsError() { +public void testUnionWithSingleSubsearchThrowsError() throws IOException { + ensureCalcsProvisioned(); assertErrorContains( "| union [search source=" + CALCS.indexName + "]", "Union command requires at least two datasets" ); } Suggestion importance[1-10]: 8 __ Why: This is a valid bug fix. The test testUnionWithSingleSubsearchThrowsError references CALCS.indexName but doesn't call ensureCalcsProvisioned(), which could cause the test to fail because the index doesn't exist rather than validating the expected error message. This inconsistency with other tests could lead to flaky test behavior.	Medium
	Add thread-safety to dataset provisioning The static calcsProvisioned flag is not thread-safe and can cause race conditions in parallel test execution. Multiple threads could simultaneously check the flag, leading to duplicate provisioning attempts or incomplete initialization visibility across threads. sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/UnionCommandIT.java [47-54] -private static boolean calcsProvisioned = false; +private static volatile boolean calcsProvisioned = false; -private void ensureCalcsProvisioned() throws IOException { +private synchronized void ensureCalcsProvisioned() throws IOException { if (calcsProvisioned == false) { DatasetProvisioner.provision(client(), CALCS); calcsProvisioned = true; } } Suggestion importance[1-10]: 5 __ Why: While the suggestion correctly identifies a potential thread-safety issue with the static calcsProvisioned flag, integration tests typically run sequentially within a test class. The volatile and synchronized additions would improve thread-safety if parallel execution is enabled, but this is a moderate concern for typical test execution patterns.	Low

---

Previous suggestions

Suggestions up to commit 33169e3

Category	Suggestion	Impact
Possible issue	Add thread-safe dataset provisioning The static calcsProvisioned flag is not thread-safe and can cause race conditions in parallel test execution. Multiple threads could simultaneously check the flag, see false, and provision the dataset multiple times. Use a thread-safe mechanism like AtomicBoolean or synchronize the method to ensure the dataset is provisioned exactly once. sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/UnionCommandIT.java [47-54] -private static boolean calcsProvisioned = false; +private static final AtomicBoolean calcsProvisioned = new AtomicBoolean(false); private void ensureCalcsProvisioned() throws IOException { - if (calcsProvisioned == false) { + if (calcsProvisioned.compareAndSet(false, true)) { DatasetProvisioner.provision(client(), CALCS); - calcsProvisioned = true; } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential race condition with the static calcsProvisioned flag in concurrent test execution. Using AtomicBoolean with compareAndSet is a valid thread-safety improvement. However, the score is moderate because test frameworks typically run test methods sequentially within a class, and the actual impact depends on the test execution configuration.	Medium

Suggestions up to commit 3982f72

Category	Suggestion	Impact
Possible issue	Add thread-safety to dataset provisioning The static calcsProvisioned flag is not thread-safe and can cause race conditions in parallel test execution. Multiple threads could simultaneously check the flag, leading to duplicate provisioning attempts or incomplete initialization visibility across threads. sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/UnionCommandIT.java [47-54] -private static boolean calcsProvisioned = false; +private static volatile boolean calcsProvisioned = false; -private void ensureCalcsProvisioned() throws IOException { +private synchronized void ensureCalcsProvisioned() throws IOException { if (calcsProvisioned == false) { DatasetProvisioner.provision(client(), CALCS); calcsProvisioned = true; } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential thread-safety issue with the static calcsProvisioned flag. Adding volatile and synchronized would prevent race conditions during parallel test execution. However, the impact depends on whether tests actually run in parallel in this test suite.	Medium
General	Ensure dataset provisioning before validation test This test method doesn't call ensureCalcsProvisioned() before executing the query, unlike all other test methods. While the query is expected to fail, the dataset should still be provisioned to ensure consistent test setup and avoid potential issues if the error check happens after dataset access. sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/UnionCommandIT.java [129-134] -public void testUnionWithSingleSubsearchThrowsError() { +public void testUnionWithSingleSubsearchThrowsError() throws IOException { + ensureCalcsProvisioned(); assertErrorContains( "| union [search source=" + CALCS.indexName + "]", "Union command requires at least two datasets" ); } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an inconsistency where testUnionWithSingleSubsearchThrowsError() doesn't call ensureCalcsProvisioned() while other tests do. Adding it ensures consistent test setup, though the test may still work without it since validation might occur before dataset access.	Low

[github-actions]

❕ Gradle check result for 3982f72: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.47%. Comparing base (d57367f) to head (42caf0f).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21649      +/-   ##
============================================
+ Coverage     73.41%   73.47%   +0.05%     
- Complexity    75533    75582      +49     
============================================
  Files          6035     6035              
  Lines        342710   342721      +11     
  Branches      49298    49301       +3     
============================================
+ Hits         251610   251808     +198     
+ Misses        71111    70925     -186     
+ Partials      19989    19988       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 33169e3

[github-actions]

✅ Gradle check result for 33169e3: SUCCESS

[ahkcs]

sandbox-check failures are pre-existing flakes on upstream/main, not caused by this PR

Both sandbox-check runs on this branch failed with the same set of analytics-engine-coordinator:internalClusterTest and composite-engine:internalClusterTest failures. I reproduced them locally against the exact base commit debf6edcad0 (i.e. upstream/main without my IT cherry-picked):

Run	Module	Failures
Rebased branch (this PR, run 26114136971)	analytics-engine-coordinator:internalClusterTest	8 (3× SearchCancellationIT, 5× CoordinatorTransportStressIT) + a few more
Rebased branch (retrigger, run 26119419897)	same	same 8 + CoordinatorSingleNodeTopologyIT, CoordinatorTwoNodeTopologyIT, WindowSqlIT, ValuesSqlIT, CompositeLocalRecoveryIT
Base commit debf6edcad0 with no IT cherry-picked (local)	same	same 8 failures, identical test names
UnionCommandIT (this PR, the only file changed)	analytics-engine-rest:integTest	7 / 7 pass ✅

All failures share the same root cause: IllegalStateException: Allocator[ROOT] closed with outstanding child allocators — an Arrow allocator lifecycle issue at JVM-test teardown. The failures vary slightly across runs (which is the hallmark of a flake), but the same suites surface consistently.

This PR only adds one file under sandbox/qa/analytics-engine-rest, a different gradle module from the failing one, and touches no allocator / coordinator code. There is no causal path from the IT change to these failures.

sandbox-check on the same base commit debf6edcad0 was green in upstream CI (run), so these tests pass intermittently on main too.

Happy to retry CI again if helpful, but absent an upstream fix for the allocator-lifecycle flake the PR has no path to a clean sandbox-check. Could you take a look or rerun the job on the merge queue? UnionCommandIT itself passes 7/7 both locally and in CI.

[github-actions]

Persistent review updated to latest commit 42caf0f

[github-actions]

✅ Gradle check result for 42caf0f: SUCCESS