Carry CalciteEvalCommandIT through the helper-managed index path

[ahkcs]

Description

Two things this PR carries together (commit-separated for clean review):

1. Carry CalciteEvalCommandIT through the helper-managed index path (28a58536d) — provisions the inline test_eval index via TestUtils.createIndexByRestClient instead of direct PUT /test_eval/_doc/N, so the analytics-engine compatibility run (tests.analytics.parquet_indices=true) gets a parquet-backed index. Mirrors the helper-managed pattern that CalciteFillNullCommandIT and CalcitePPLAppendCommandIT already use via loadIndex(Index.X).

2. Workaround for Gradle 9.4.1 TestEventReporterAsListener cast bug (435827e99) — excludes @Ignore-annotated test classes from integTest so the task no longer aborts mid-run on this branch's CI.

---

1. Helper-managed test_eval provisioning

Why this matters

When the analytics-engine compatibility path is active (tests.analytics.force_routing=true and tests.analytics.parquet_indices=true), every test-created index needs parquet-backed storage so the DataFusion backend can scan it — TestUtils.makeParquetBacked handles the injection inside createIndexByRestClient. Direct PUT /test_eval/_doc/N calls bypass that helper, so the index lands Lucene-backed and the analytics planner rejects every query with No backend can scan all requested fields on index [test_eval].

The three tests that use test_eval fail on the analytics route today; only testEvalStringConcatenationWithExistingData (which uses loadIndex(Index.BANK)) survives. With this change, test_eval is created via the same helper as every other fixture and inherits the parquet flag automatically.

The companion analytics-engine wiring (CONCAT/CAST/SAFE_CAST capabilities + null-propagating concat adapter) lands separately in opensearch-project/OpenSearch#21498. On its own, this PR is a no-op for the v2 / Calcite path — once both this and the core PR are merged, CalciteEvalCommandIT goes from 1/4 to 4/4 on the analytics route.

Changes

1. Helper-managed test_eval provisioning. init() now calls TestUtils.createIndexByRestClient(client(), "test_eval", testEvalMapping) with an explicit mapping (name/title = keyword, age = long). The helper is a thin wrapper around the index PUT on the v2 path (no-op when tests.analytics.parquet_indices is unset), so v2 behaviour is unchanged.

2. Idempotency guard. The whole init body is wrapped in TestUtils.isIndexExist(client(), "test_eval") — same pattern loadIndex uses for predefined fixtures. init() runs before every @Test method; first run provisions, subsequent runs skip. Without the guard, createIndexByRestClient (unlike the original doc-level PUT) would throw resource_already_exists_exception on the second invocation.

3. Pinned projection order on testEvalStringConcatenation. The original query had no | fields clause and relied on the implicit projection's column order, which differs between paths (v2 returns _source-iteration order, analytics returns parquet-storage order). Added | fields name, title, age, greeting to make the assertion deterministic. Expected rows already match this order, so v2 behaviour is preserved.

V2-path impact

Surface	Before	After
name PPL type	string (text → string)	string (keyword → string)
age PPL type	bigint	bigint
title PPL type	string	string
Eval read source	_source	_source
Concat result	"Hello Alice"	"Hello Alice"
Test 1 expected row order	[name, title, age, greeting] (matched by coincidence)	[name, title, age, greeting] (now pinned)

verifySchema(schema("name", "string")) matches both text+keyword (dynamic) and keyword (explicit) — PPL maps both to the same logical type. No assertion changes were needed.

---

2. Gradle 9.4.1 @Ignore cast workaround

Why this is in this PR

CI on this branch (post-#5406 wrapper bump to 9.4.1) aborts the :integ-test:integTest task with:

Test process encountered an unexpected problem.
> class org.gradle.api.internal.tasks.testing.LifecycleTrackingTestEventReporter cannot be cast to class
  org.gradle.api.internal.tasks.testing.GroupTestEventReporterInternal

…even though no test assertion fails. Without a workaround, this PR's tests would be invisible — the task crashes before final result aggregation. Folding the workaround into this PR unblocks our CI; the workaround is a base-branch concern but the symptom blocks every PR until landed.

Root cause (verified via javap against gradle-testing-base-9.4.1.jar)

• org/gradle/api/tasks/testing/AbstractTestTask (line 263) instantiates TestEventReporterAsListener — Gradle's internal bridge from TestListenerInternal to the new TestEventReporter API.
• TestEventReporterAsListener.started line 58 stores a per-descriptor reporter in a map keyed by id and, on each child event, casts the parent's stored reporter to GroupTestEventReporterInternal.
• A class-level @Ignore produces a non-composite parent descriptor with a leaf TestEventReporter rather than a group reporter — the cast fails. First failure in CI: CalciteInformationSchemaCommandIT.testTablesFromPrometheusCatalog (that class is @Ignore'd for [FEATURE] Support Prometheus DataSource with Calcite #3455).
• The bridge is instantiated by Gradle's own AbstractTestTask, not by OpenSearchTestBasePlugin.addTestListener(ErrorReportingTestListener). Migrating ErrorReportingTestListener to the new event-reporter API would not stop the bridge from firing.

Workaround

Exclude all @Ignore-annotated test classes from integTest. The classes were already skipped at the JUnit layer; this just moves the skip earlier (build layer) so the buggy bridge never sees them.

	Before	After
@Ignore'd test runtime status	Skipped (JUnit)	Not run (Gradle exclude)
Pass/fail outcome	Skipped	Skipped (effectively — same as not-run for CI gating)
Skipped-test count in dashboards	Includes @Ignore'd	One-time delta (excludes them)
integTest task aborts mid-run	❌ yes (cast crash)	✅ no
testLogging.events "failed" per-test message	Preserved	Preserved

24 classes excluded (one — OrderIT — was already excluded by the existing block). Source: grep -rln '^@Ignore' integ-test/src/test/java minus already-excluded paths.

This block should be removed once Gradle ships a fix for the bridge cast. Given the bug is in Gradle's own internal class, the proper long-term fix is upstream — file an issue on gradle/gradle referencing the TestEventReporterAsListener.started:58 site if not already filed.

---

Test results

	v2 path (this PR alone)	analytics route (this PR + core #21498)
testEvalStringConcatenation	passes	passes
testEvalStringConcatenationWithNullField	passes	passes
testEvalStringConcatenationWithLiterals	passes	passes
testEvalStringConcatenationWithExistingData	passes	passes
Analytics-route status	unchanged (1/4)	4/4
:integ-test:integTest task	crashes pre-PR (Gradle 9.4.1 bug)	runs to completion

Forward note

The same direct-PUT pattern that this PR fixes for CalciteEvalCommandIT exists in roughly a dozen other Calcite ITs (CalcitePPLAggregationIT, CalcitePPLBasicIT, CalciteFieldFormatCommandIT, CalcitePPLCaseFunctionIT, etc.). Each will need the same one-line carryover when its corresponding command lands on the analytics route. This PR addresses only the eval surface.

By submitting this pull request

• All commits are signed per the DCO.
• My code follows the code of conduct of this project.
• My change requires changes to the documentation. — N/A (test-infra change)
• I have updated the documentation accordingly. — N/A
• I have added tests to cover my changes. — Existing tests; this PR carries them through the helper-managed path so they survive on the analytics route.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 435827e)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Idempotency Risk The isIndexExist guard prevents re-creating the index, but if a previous test run left the index in a partial or corrupt state (e.g., only the mapping was created but no documents were inserted), subsequent runs will skip document insertion entirely. Consider adding a cleanup/delete step before creation, or ensuring the index is deleted in a @AfterAll/teardown method. if (!TestUtils.isIndexExist(client(), "test_eval")) { String testEvalMapping = "{\"mappings\":{\"properties\":{" + "\"name\":{\"type\":\"keyword\"}," + "\"age\":{\"type\":\"long\"}," + "\"title\":{\"type\":\"keyword\"}}}}"; TestUtils.createIndexByRestClient(client(), "test_eval", testEvalMapping); // Create test data for string concatenation Request request1 = new Request("PUT", "/test_eval/_doc/1?refresh=true"); request1.setJsonEntity("{\"name\": \"Alice\", \"age\": 25, \"title\": \"Engineer\"}"); client().performRequest(request1); Request request2 = new Request("PUT", "/test_eval/_doc/2?refresh=true"); request2.setJsonEntity("{\"name\": \"Bob\", \"age\": 30, \"title\": \"Manager\"}"); client().performRequest(request2); Request request3 = new Request("PUT", "/test_eval/_doc/3?refresh=true"); request3.setJsonEntity("{\"name\": \"Charlie\", \"age\": null, \"title\": \"Analyst\"}"); client().performRequest(request3); } Null Age Handling Document 3 sets "age": null explicitly. Since the mapping defines age as long, inserting a null value may behave differently across Lucene and parquet-backed engines. Verify that both execution paths handle null long values consistently, especially in the verifySchema and verifyDataRows assertions downstream. Request request3 = new Request("PUT", "/test_eval/_doc/3?refresh=true"); request3.setJsonEntity("{\"name\": \"Charlie\", \"age\": null, \"title\": \"Analyst\"}"); client().performRequest(request3);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 435827e
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard condition may leave index empty on partial failure The document PUT requests inside the if block do not use ?refresh=true in a way that guarantees all three documents are visible before the test queries run, since each refresh only covers that single document. More critically, if the index already exists from a previous test run but is empty (e.g., due to a partial failure), the guard isIndexExist will skip re-inserting the documents, leaving the index empty and causing test failures. Consider separating the existence check for the index from the data population, or deleting and recreating the index unconditionally in init(). integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteEvalCommandIT.java [37-57] -if (!TestUtils.isIndexExist(client(), "test_eval")) { - String testEvalMapping = - "{\"mappings\":{\"properties\":{" - + "\"name\":{\"type\":\"keyword\"}," - + "\"age\":{\"type\":\"long\"}," - + "\"title\":{\"type\":\"keyword\"}}}}"; - TestUtils.createIndexByRestClient(client(), "test_eval", testEvalMapping); +TestUtils.createIndexByRestClient(client(), "test_eval", testEvalMapping); - // Create test data for string concatenation - Request request1 = new Request("PUT", "/test_eval/_doc/1?refresh=true"); - request1.setJsonEntity("{\"name\": \"Alice\", \"age\": 25, \"title\": \"Engineer\"}"); - client().performRequest(request1); +Request request1 = new Request("PUT", "/test_eval/_doc/1?refresh=true"); +request1.setJsonEntity("{\"name\": \"Alice\", \"age\": 25, \"title\": \"Engineer\"}"); +client().performRequest(request1); - Request request2 = new Request("PUT", "/test_eval/_doc/2?refresh=true"); - request2.setJsonEntity("{\"name\": \"Bob\", \"age\": 30, \"title\": \"Manager\"}"); - client().performRequest(request2); +Request request2 = new Request("PUT", "/test_eval/_doc/2?refresh=true"); +request2.setJsonEntity("{\"name\": \"Bob\", \"age\": 30, \"title\": \"Manager\"}"); +client().performRequest(request2); - Request request3 = new Request("PUT", "/test_eval/_doc/3?refresh=true"); - request3.setJsonEntity("{\"name\": \"Charlie\", \"age\": null, \"title\": \"Analyst\"}"); - client().performRequest(request3); -} +Request request3 = new Request("PUT", "/test_eval/_doc/3?refresh=true"); +request3.setJsonEntity("{\"name\": \"Charlie\", \"age\": null, \"title\": \"Analyst\"}"); +client().performRequest(request3); Suggestion importance[1-10]: 4 __ Why: The concern about partial failures leaving an empty index is valid but somewhat theoretical for integration tests that typically run in clean environments. The refresh=true on each individual PUT is actually sufficient for visibility since each document is immediately refreshed. The suggestion to remove the guard entirely contradicts the PR's explicit intent to support idempotent init() calls for the analytics-engine compatibility run, making the improved_code incomplete as it omits the mapping variable and index creation guard logic.	Low

---

Previous suggestions

Suggestions up to commit 28a5853

Category	Suggestion	Impact
General	Consolidate refresh after all document inserts The documents are inserted without a final ?refresh=true barrier after all three PUTs, but more critically, when the index already exists (the else branch is skipped), the test proceeds without verifying that the expected documents are actually present. If a previous test run created the index but failed mid-population, subsequent runs will silently operate on incomplete data. Consider adding a document count check or always ensuring all three documents exist regardless of the index-existence guard. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteEvalCommandIT.java [37-57] if (!TestUtils.isIndexExist(client(), "test_eval")) { String testEvalMapping = "{\"mappings\":{\"properties\":{" + "\"name\":{\"type\":\"keyword\"}," + "\"age\":{\"type\":\"long\"}," + "\"title\":{\"type\":\"keyword\"}}}}"; TestUtils.createIndexByRestClient(client(), "test_eval", testEvalMapping); - // Create test data for string concatenation - Request request1 = new Request("PUT", "/test_eval/_doc/1?refresh=true"); + Request request1 = new Request("PUT", "/test_eval/_doc/1"); request1.setJsonEntity("{\"name\": \"Alice\", \"age\": 25, \"title\": \"Engineer\"}"); client().performRequest(request1); - Request request2 = new Request("PUT", "/test_eval/_doc/2?refresh=true"); + Request request2 = new Request("PUT", "/test_eval/_doc/2"); request2.setJsonEntity("{\"name\": \"Bob\", \"age\": 30, \"title\": \"Manager\"}"); client().performRequest(request2); - Request request3 = new Request("PUT", "/test_eval/_doc/3?refresh=true"); + Request request3 = new Request("PUT", "/test_eval/_doc/3"); request3.setJsonEntity("{\"name\": \"Charlie\", \"age\": null, \"title\": \"Analyst\"}"); client().performRequest(request3); + + // Refresh once after all documents are indexed + client().performRequest(new Request("POST", "/test_eval/_refresh")); } Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about consolidating the refresh operation, but the improved_code actually removes ?refresh=true from individual requests and adds a single _refresh call at the end — this is a minor optimization. The concern about incomplete data if a previous run failed mid-population is valid but unlikely in practice, and the suggestion doesn't fully address that concern in the improved_code.	Low

[github-actions]

Persistent review updated to latest commit 435827e