[BugFix] Scope SQL cursor continuation to original query indices under FGAC by mengweieric · Pull Request #5399 · opensearch-project/sql

mengweieric · 2026-05-01T01:22:38Z

Summary

Legacy (V1) SQL cursor continuation built its SearchRequest with new SearchRequest() (no indices). Under OpenSearch Security Fine-Grained Access Control, an indices-less SearchRequest resolves to a wildcard during authorization, so users who only have permissions on the queried index get 403 no permissions for [indices:data/read/search] on page 2 and later. Page 1 worked because the initial SearchRequest was built via SearchRequestBuilder and carried the concrete indices.
Persist the resolved indices into DefaultCursor at page-1 time and scope the continuation SearchRequest with them. Security now authorizes against the same concrete indices across every page.

Root cause

PrettyFormatRestExecutor.buildProtocolWithPagination creates the page-1 SearchRequest via queryAction.getRequestBuilder(), which carries queryAction.getSelect().getIndexArr(). Security sees concrete indices and grants.
On continuation, CursorResultExecutor.handleDefaultCursorRequest built new SearchRequest() with no indices and only a SearchSourceBuilder + PIT id. Security resolves the empty indices array to the cluster-wide wildcard, evaluates the user's role against *, and denies.

The PIT is correctly scoped on both page 1 and continuation; the denial happens in Security's pre-PIT authorization of the outer SearchRequest.

Fix

DefaultCursor: new String[] indices field, serialized under a new short cursor-payload key "x". Deserialization uses optJSONArray and defaults to new String[0] when the key is absent, so cursor tokens issued by pre-fix nodes continue to deserialize cleanly after a rolling upgrade.
PrettyFormatRestExecutor.createCursorWithPit: populates cursor.setIndices(queryAction.getSelect().getIndexArr()) at page-1 time.
CursorResultExecutor.handleDefaultCursorRequest: continuation SearchRequest now new SearchRequest(cursor.getIndices()).

Scope: only the legacy V1 cursor path changes. The V2 cursor path (buildProtocolWithPagination going through SearchRequestBuilder.get()) already carried indices end-to-end and is untouched; PaginationIT stays green.

Back-compat

Old in-flight cursor tokens (no "x" field) deserialize with indices=[]. Those continuations still hit the original bug for FGAC users but behave exactly as they did before the fix, i.e. nothing gets worse during a rolling upgrade.
New cursor tokens written by upgraded nodes contain "x" and authorize correctly on every page.
Non-FGAC clusters are unaffected in either direction.

Test plan

./gradlew :legacy:spotlessCheck :integ-test:spotlessCheck
./gradlew :legacy:test (all pass, including 4 new DefaultCursorTest cases: serialization of indices, null-indices default, round-trip, legacy-cursor-without-x back-compat)
./gradlew build -x integTest -x integTestWithSecurity -x yamlRestTest (all module unit tests)
./gradlew :integ-test:integTestWithSecurity --tests org.opensearch.sql.security.SQLCursorPermissionsIT (both tests pass)
Verified the new IT catches the bug: with the three source edits stashed, cursorPaginationUnderFgacSucceedsAcrossPages fails with the exact customer symptom (403 no permissions for [indices:data/read/search]), then passes once the fix is restored.
./gradlew :integ-test:integTest --tests org.opensearch.sql.legacy.CursorIT (54/54 pass; serialization change is back-compat with existing V1 cursor flows)
./gradlew :integ-test:integTest --tests org.opensearch.sql.sql.PaginationIT (24/24 pass; V2 cursor path unaffected)

Files changed

legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java
legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java
legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java
legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java
integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java (new)

…r FGAC The legacy (V1) SQL cursor continuation path built its SearchRequest with `new SearchRequest()` (no indices). Under OpenSearch Security Fine-Grained Access Control, an indices-less SearchRequest resolves to a wildcard during authorization, so users who only have permissions on the queried index are denied `indices:data/read/search` on the second and subsequent pages. Page 1 worked because the initial SearchRequest was built via SearchRequestBuilder and carried the concrete indices. Persist the resolved indices into DefaultCursor at page-1 time and pass them to the continuation SearchRequest so Security authorizes against the same concrete indices across all pages. The cursor payload uses a new short key 'x' and falls back to an empty array when absent, so cursor tokens issued by pre-fix nodes continue to deserialize cleanly. Regression coverage: - Unit test for DefaultCursor round-trip including the legacy-without-x case. - Integration test SQLCursorPermissionsIT that exercises V1 cursor paging under an FGAC role scoped to a single index. The new test fails on HEAD with 403 on page 2 and passes after the fix. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>

github-actions · 2026-05-01T01:23:57Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes Sub-PR theme: Core FGAC cursor fix with unit tests Relevant files: legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java Sub-PR theme: Integration test for SQL cursor FGAC regression Relevant files: integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java
⚡ Recommended focus areas for review Test Reliability The `initialized` flag is an instance field, but `SQLIntegTestCase` may create a new instance per test method. If so, `createSecurityRolesAndUsers()` will be called multiple times, and the guard `if (initialized) return;` will never fire. This could cause redundant role/user creation requests or failures if the security API returns an error on duplicate creation (rather than 200/201). Consider using a `@BeforeClass` static flag or `assumeTrue` to guard idempotent setup. private boolean initialized = false; @Override protected void init() throws Exception { loadIndex(Index.ACCOUNT); createSecurityRolesAndUsers(); } private void createSecurityRolesAndUsers() throws IOException { if (initialized) { return; } createRole(ACCOUNT_ROLE, TEST_INDEX_ACCOUNT); createUser(ACCOUNT_USER, ACCOUNT_ROLE); initialized = true; } Hardcoded Row Count The test asserts exactly 5 pages based on 234 rows / 50 per page. This assumes the `TEST_INDEX_ACCOUNT` index always contains exactly 1000 rows (or at least 234). If the index has fewer rows, the cursor may exhaust earlier and the assertion will fail. The test should either assert `pages >= 2` (to confirm at least one continuation page succeeded) or document the exact expected row count more explicitly. // 234 rows / 50 per page = 5 pages assertEquals("expected 5 V1 cursor pages under FGAC", 5, pages); } Charset Assumption In the legacy-cursor backward-compatibility test, `json.toString().getBytes()` and `new String(Base64.getDecoder().decode(payload))` use the platform default charset. If the platform charset is not UTF-8, encoding/decoding may be inconsistent. Explicit `StandardCharsets.UTF_8` should be used. The same concern applies to `decodePayload` in the test. } public static DefaultCursor from(String cursorId) { Empty Indices Fallback When `cursor.getIndices()` returns an empty array (e.g., for cursors issued by pre-fix nodes after a rolling upgrade), the `SearchRequest` is constructed with `new SearchRequest(new String[0])`. Depending on the OpenSearch version, an empty indices array may still resolve to a wildcard, meaning the FGAC issue could persist for in-flight cursors from pre-fix nodes. This edge case should be documented or handled explicitly. String[] indices = cursor.getIndices(); SearchRequest searchRequest = new SearchRequest(indices == null ? new String[0] : indices);

github-actions · 2026-05-01T01:24:25Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fallback to index pattern for legacy cursors When `indices` is an empty array (e.g., deserialized from a legacy cursor without the "x" field), the `SearchRequest` will still be constructed with no indices, which resolves to a wildcard under FGAC and causes the same 403 denial the fix intends to prevent. Consider falling back to `cursor.getIndexPattern()` when the indices array is empty, so legacy cursors still work correctly after upgrade. legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java [120-121] String[] indices = cursor.getIndices(); -SearchRequest searchRequest = new SearchRequest(indices == null ? new String[0] : indices); +SearchRequest searchRequest; +if (indices != null && indices.length > 0) { + searchRequest = new SearchRequest(indices); +} else { + // Fallback for legacy cursors that predate the indices field + searchRequest = new SearchRequest(cursor.getIndexPattern().split("\\\|")); +} Suggestion importance[1-10]: 7 __ Why: This is a valid concern: when `indices` is empty (legacy cursor without "x" field), the fix still creates a `SearchRequest` with no indices, which would still fail under FGAC. Falling back to `indexPattern` would handle the upgrade scenario correctly. However, the test `deserializeLegacyCursorWithoutIndicesDefaultsToEmptyArray` explicitly tests that empty array is returned for legacy cursors, suggesting the author may have intended a different fallback strategy.	Medium
Possible issue	Safely encode cursor value in JSON body The cursor string may contain characters that are not safe to embed directly in a JSON string literal (e.g., `+`, `/`, `=` from Base64, or special characters). Using `String.format` to inject the cursor value without escaping can produce malformed JSON and cause unexpected failures. Use a `JSONObject` to build the request body so the cursor value is properly escaped. integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java [181-190] int pages = 1; while (!cursor.isEmpty()) { - JSONObject next = - executeSqlAsUser( - String.format(Locale.ROOT, "{\"cursor\": \"%s\"}", cursor), ACCOUNT_USER); + JSONObject body = new JSONObject(); + body.put("cursor", cursor); + JSONObject next = executeSqlAsUser(body.toString(), ACCOUNT_USER); cursor = next.optString("cursor", ""); pages++; } // 234 rows / 50 per page = 5 pages assertEquals("expected 5 V1 cursor pages under FGAC", 5, pages); Suggestion importance[1-10]: 6 __ Why: The cursor value (Base64-encoded) can contain characters like `+`, `/`, and `=` that are safe in JSON strings but the suggestion is valid for robustness. Using `JSONObject` to build the request body ensures proper escaping and avoids potential malformed JSON issues.	Low
General	Use explicit charset for Base64 encode/decode `json.toString().getBytes()` uses the platform's default charset, which may differ across environments and cause decoding failures. Explicitly specify `StandardCharsets.UTF_8` for both encoding and decoding to ensure consistent behavior. legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java [170-172] -JSONObject json = new JSONObject(new String(Base64.getDecoder().decode(payload))); +JSONObject json = new JSONObject(new String(Base64.getDecoder().decode(payload), java.nio.charset.StandardCharsets.UTF_8)); json.remove("x"); -String legacyPayload = Base64.getEncoder().encodeToString(json.toString().getBytes()); +String legacyPayload = Base64.getEncoder().encodeToString(json.toString().getBytes(java.nio.charset.StandardCharsets.UTF_8)); Suggestion importance[1-10]: 4 __ Why: Using explicit `StandardCharsets.UTF_8` is a good practice to avoid platform-dependent behavior, but this is a test file and the risk of charset mismatch causing failures in practice is low. It's a minor improvement to code robustness.	Low

@ignore

Bring the analytics-engine catch-up PR up to current upstream/main by resolving conflicts introduced by 4 main commits since 2026-04-30: - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) Resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. PR's RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept HEAD's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking. Same pattern as the original PR resolution; both improvements are orthogonal. plugin/SQLPlugin.java: kept HEAD imports for ExplainResponse and QueryType (referenced by createSqlAnalyticsRouter, which only exists on the feature branch). plugin/transport/TransportPPLQueryAction.java: kept HEAD's queryPlanExecutor parameter alongside main's extensionsHolder parameter, since both are referenced in the constructor body. legacy/plugin/RestSqlAction.java: took HEAD. The 3-way merge produced a duplicated handleException/getRawErrorCode block; HEAD already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so HEAD is the correct superset. Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava and unit tests pass; spotlessCheck passes. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

…ation Brings the catch-up branch up to current upstream/main (4 commits since this PR was opened) and current feature/mustang-ppl-integration (9 commits since this PR was opened), so the PR is mergeable into feature/mustang-ppl-integration without conflicts. Squashed (rather than two real merge commits) for the same DCO reason the original commit was squashed: upstream commits authored by many contributors with inconsistent or missing Signed-off-by trailers would otherwise be brought into this PR's history. Newer main commits absorbed (4): - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) Newer feature commits absorbed (9): - opensearch-project#5403 (analytics-engine optional dependency — major rewiring) - opensearch-project#5407 (Carry CalciteEvalCommandIT through helper-managed index path) - opensearch-project#5413 (Default plugins.calcite.enabled=true on unified path) - opensearch-project#5415, opensearch-project#5416, opensearch-project#5417, opensearch-project#5409, opensearch-project#5400, opensearch-project#5406 (smaller carryovers + bumps) Conflict resolutions (10 from main side, 3 from feature side): api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. PR's RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept HEAD's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking. Same pattern as the original PR resolution; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took HEAD. The 3-way merge produced a duplicated handleException/getRawErrorCode block; HEAD already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so HEAD is the correct superset. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 TestEventReporterAsListener cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from opensearch-project#5407) so analytics-engine compatibility runs get a parquet-backed index. Added back PR HEAD's test_eval_agent setup (needed by the dotted-path eval tests for opensearch-project#5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java: took feature. PR opensearch-project#5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter. Feature's design supersedes our prior wiring. plugin/.../SQLPlugin.java: took feature. The same opensearch-project#5403 simplification removed loadExtensions/EngineExtensionsHolder/executionEngineExtensions plumbing (no longer needed once analytics-engine is optionally bound). Feature retains the createSqlAnalyticsRouter method this PR introduced. plugin/.../config/EngineExtensionsHolder.java: deleted. Unreferenced after taking feature's SQLPlugin/TransportPPLQueryAction; not present on feature branch. Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

Single squashed commit on top of feature/mustang-ppl-integration that absorbs upstream/main's commits not yet on the feature branch. Replaces the prior catch-up squash (opensearch-project#5396 base + the original af831d3 rebase commit) so this PR is a fast-forward into feature/mustang-ppl-integration. Squashed (rather than a merge commit) because upstream main commits were authored by many contributors with inconsistent or missing Signed-off-by trailers; DCO would otherwise reject those commits. Main commits absorbed (54 since divergence; 4 since the original catch-up squash was made on 2026-04-30): - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) - opensearch-project#5394 (SQL Vector Search), opensearch-project#5361 (OpenSearch 3.7), opensearch-project#5360 (unified SQL language spec), opensearch-project#5240 (PPL Union), and 46 others. Conflict resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept feature's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took feature. The 3-way merge produced a duplicated handleException/getRawErrorCode block; feature already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so feature is the correct superset. CLAUDE.md, docs/user/ppl/functions/condition.md: took main. explain_streamstats_global{,_null_bucket}.yaml: took main (post-opensearch-project#5359 shape). core/CalciteRelNodeVisitor + utils/PlanUtils: took main (collation utility hoisted from CalciteRelNodeVisitor.backtrackForCollation into PlanUtils.findInputCollation). integ-test/CalciteNoPushdownIT.java: added CalciteMixedFieldTypeIT. ppl/antlr/OpenSearchPPLParser.g4: added unionCommand. ppl/calcite/CalcitePPLStreamstatsTest.java: added testMultipleStreamstatsWithWindow. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from opensearch-project#5407) so analytics-engine compatibility runs get a parquet-backed index. Added the test_eval_agent setup (needed by the dotted-path eval tests for opensearch-project#5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java, plugin/.../SQLPlugin.java: took feature. PR opensearch-project#5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter, and removed the loadExtensions / EngineExtensionsHolder / executionEngineExtensions plumbing. Feature retains the createSqlAnalyticsRouter method this catch-up introduced. plugin/.../config/EngineExtensionsHolder.java: deleted (unreferenced post-opensearch-project#5403; not present on feature). Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

…5397) Single squashed commit on top of feature/mustang-ppl-integration that absorbs upstream/main's commits not yet on the feature branch. Replaces the prior catch-up squash (#5396 base + the original af831d3 rebase commit) so this PR is a fast-forward into feature/mustang-ppl-integration. Squashed (rather than a merge commit) because upstream main commits were authored by many contributors with inconsistent or missing Signed-off-by trailers; DCO would otherwise reject those commits. Main commits absorbed (54 since divergence; 4 since the original catch-up squash was made on 2026-04-30): - #5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - #5408 (datetime type normalization) - #5414 (Gradle wrapper bump + @ignore exclusion) - #5399 (FGAC-scoped SQL cursor continuation) - #5394 (SQL Vector Search), #5361 (OpenSearch 3.7), #5360 (unified SQL language spec), #5240 (PPL Union), and 46 others. Conflict resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in #5408 / #5419. core/executor/QueryService.java: composed both sides — kept feature's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took feature. The 3-way merge produced a duplicated handleException/getRawErrorCode block; feature already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so feature is the correct superset. CLAUDE.md, docs/user/ppl/functions/condition.md: took main. explain_streamstats_global{,_null_bucket}.yaml: took main (post-#5359 shape). core/CalciteRelNodeVisitor + utils/PlanUtils: took main (collation utility hoisted from CalciteRelNodeVisitor.backtrackForCollation into PlanUtils.findInputCollation). integ-test/CalciteNoPushdownIT.java: added CalciteMixedFieldTypeIT. ppl/antlr/OpenSearchPPLParser.g4: added unionCommand. ppl/calcite/CalcitePPLStreamstatsTest.java: added testMultipleStreamstatsWithWindow. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from #5407) so analytics-engine compatibility runs get a parquet-backed index. Added the test_eval_agent setup (needed by the dotted-path eval tests for #5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java, plugin/.../SQLPlugin.java: took feature. PR #5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter, and removed the loadExtensions / EngineExtensionsHolder / executionEngineExtensions plumbing. Feature retains the createSqlAnalyticsRouter method this catch-up introduced. plugin/.../config/EngineExtensionsHolder.java: deleted (unreferenced post-#5403; not present on feature). Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

mengweieric added bug Something isn't working SQL skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels May 1, 2026

mengweieric added the enhancement New feature or request label May 1, 2026

mengweieric marked this pull request as ready for review May 1, 2026 01:35

mengweieric requested review from LantaoJin, RyanL1997, Swiddis, acarbonetto, ahkcs, anirudha, dai-chen, joshuali925, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners May 1, 2026 01:35

Swiddis approved these changes May 1, 2026

View reviewed changes

RyanL1997 approved these changes May 5, 2026

View reviewed changes

RyanL1997 merged commit c0ac784 into opensearch-project:main May 5, 2026
43 of 48 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BugFix] Scope SQL cursor continuation to original query indices under FGAC#5399

[BugFix] Scope SQL cursor continuation to original query indices under FGAC#5399
RyanL1997 merged 1 commit intoopensearch-project:mainfrom
mengweieric:fix/sql-cursor-fgac-indices

mengweieric commented May 1, 2026

Uh oh!

github-actions Bot commented May 1, 2026

Uh oh!

github-actions Bot commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mengweieric commented May 1, 2026

Summary

Root cause

Fix

Back-compat

Test plan

Files changed

Uh oh!

github-actions Bot commented May 1, 2026

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented May 1, 2026

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants