Skip to content

[BugFix] Fix SQL CLI integration test flaky failure#5347

Merged
Swiddis merged 1 commit into
opensearch-project:mainfrom
ahkcs:fix/sql-cli-flaky-test-5335
Apr 14, 2026
Merged

[BugFix] Fix SQL CLI integration test flaky failure#5347
Swiddis merged 1 commit into
opensearch-project:mainfrom
ahkcs:fix/sql-cli-flaky-test-5335

Conversation

@ahkcs
Copy link
Copy Markdown
Collaborator

@ahkcs ahkcs commented Apr 14, 2026 •
edited
Loading

Summary

  • Fixes intermittent NoSuchFileException in the SQL CLI integration test workflow caused by a Gradle incremental compilation race condition
  • Adds ./gradlew clean after publishToMavenLocal so sql-cli resolves dependencies from Maven Local jars instead of stale build output

Test plan

  • Verify the sql-cli-integration-test workflow passes consistently (no more flaky NoSuchFileException failures)

@ahkcs
Fix SQL CLI integration test flaky failure (opensearch-project#5335)
9512ff8 
Add `./gradlew clean` after `publishToMavenLocal` to remove stale
build output. This ensures sql-cli resolves SQL dependencies from
Maven Local jars instead of the source project build directory,
eliminating the incremental compilation race condition that caused
intermittent NoSuchFileException failures.

Signed-off-by: Kai Huang <huangkaics@gmail.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Clean Order

Running ./gradlew clean after publishToMavenLocal removes the build output directories, but the published artifacts in Maven Local (~/.m2) should remain intact. However, it's worth verifying that the clean task does not inadvertently remove or invalidate any files that the subsequent SQL CLI test step depends on, and that the published Maven Local artifacts are fully self-contained before clean is invoked.

 
./gradlew clean

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Move clean task before publish step

Running ./gradlew clean after publishToMavenLocal may delete build artifacts that
were just published to Maven Local, potentially causing issues in subsequent steps.
The clean task should be run before the publish step, not after, to avoid
interfering with the published artifacts.

.github/workflows/sql-cli-integration-test.yml [70-72]

+./gradlew clean
 ./gradlew publishToMavenLocal -x test -x integTest
 echo "SQL modules published to Maven Local"
-./gradlew clean
Suggestion importance[1-10]: 3

__

Why: While the suggestion raises a valid concern about ordering, publishToMavenLocal copies artifacts to the local Maven repository (~/.m2), not the build directory. Running ./gradlew clean after publishing would only remove the build/ directory, not the published artifacts in Maven Local, so the concern about interfering with published artifacts is likely incorrect.

Low

@ahkcs ahkcs added bugFix maintenance Improves code quality, but not the product infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. labels Apr 14, 2026
Swiddis
Swiddis approved these changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Swiddis Swiddis left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't fully understand how this fixes the problem but i'll believe it lol

@Swiddis Swiddis merged commit ee28e3d into opensearch-project:main Apr 14, 2026
48 of 49 checks passed
@ahkcs ahkcs deleted the fix/sql-cli-flaky-test-5335 branch April 14, 2026 22:42
@ahkcs ahkcs mentioned this pull request May 11, 2026
Merged
6 tasks
ahkcs added a commit to ahkcs/sql that referenced this pull request May 11, 2026
@ahkcs
Merge upstream/main into feature/mustang-ppl-integration
f1bc872 
Brings the branch up to current upstream/main (54 commits since the last
catch-up at opensearch-project#5397, divergence point 513e1b2). Preserves both parents
so commit lineage from main is retained on the feature branch.

### Main commits absorbed (54 since divergence)

Highlights:
- opensearch-project#5419 Register LENGTH/REGEXP_REPLACE/DATE_TRUNC in unified function spec
- opensearch-project#5408 Normalize datetime types for unified query API
- opensearch-project#5414 Gradle wrapper 9.4.1 bump + exclude @ignore classes
- opensearch-project#5399 [BugFix] Scope SQL cursor continuation to original query indices under FGAC
- opensearch-project#5394 [Feature] Support SQL Vector Search
- opensearch-project#5361 Version bump to OpenSearch 3.7 (Jackson 2 → 3 parser API)
- opensearch-project#5360 Define unified SQL language spec with composable extensions
- opensearch-project#5240 [FEATURE] Union command in PPL
- opensearch-project#5266 Initial implementation of report-builder interface
- opensearch-project#5278 isnotnull condition support
- opensearch-project#3922, opensearch-project#4659, opensearch-project#4800, opensearch-project#5099, opensearch-project#5169, opensearch-project#5172, opensearch-project#5175, opensearch-project#5185, opensearch-project#5347, opensearch-project#5370
  and other bugfixes
- 34 others (bugfixes, doc updates, infra)

### Conflict resolutions

Seven content conflicts resolved during the merge. Resolution kept the
feature branch's analytics-engine-path semantics where main's changes
would have regressed them.

- api/.../UnifiedQueryContext.java
  Blank-line-only conflict in Builder.build(); took main's tighter
  formatting (no extra blank).

- core/.../executor/QueryService.java
  Main reformatted executeWithCalcite without the
  CalciteClassLoaderHelper.withCalciteClassLoader() wrapper. Kept the
  feature-branch wrapping — required for analytics-engine classloader
  isolation — and the matching import. Same shape applies to
  explainWithCalcite which git auto-merged correctly.

- integ-test/build.gradle
  Both sides added @Ignore-class exclusions to work around the Gradle
  9.4.1 TestEventReporterAsListener ClassCastException. Took the
  feature-branch's detailed root-cause comment, kept ASCII ordering of
  the JSONRequestIT/JoinIT and SQLFunctionsIT/ShowIT/SourceFieldIT
  entries.

- integ-test/.../CalciteEvalCommandIT.java
  Feature branch wraps test-data PUTs in
  if (!TestUtils.isIndexExist(...)) for idempotency on the
  helper-managed-index analytics-engine compatibility run; main has the
  unguarded PUTs. Kept feature's idempotency guards for both test_eval
  and test_eval_agent.

- legacy/.../RestSqlAction.java
  Feature branch added delegateToV2Engine(...) (extracted from the
  analytics-engine routing path) and ALSO has handleException /
  getRestStatus / getRawErrorCode; main added the latter three in the
  same position. Kept feature's delegateToV2Engine, kept one copy of
  the helpers, removed the duplicate set that git produced.

- plugin/.../SQLPlugin.java
  Both sides expanded the executor import block. Kept the union:
  ExecutionEngine + ExecutionEngine.ExplainResponse + QueryType (all
  three are referenced in this file).

- plugin/.../transport/TransportPPLQueryAction.java
  Main now passes engine extensions to OpenSearchPluginModule via
  extensionsHolder.engines(). Feature creates a local pluginSettings
  for the unified-query handler. Combined both — kept the
  extensions-aware module construction *and* the pluginSettings /
  pluginSettingsRef wiring.

EngineExtensionsHolder is a new file from main (opensearch-project#5298) preserved as-is.

### Verification

- ./gradlew :api:compileJava :core:compileJava :legacy:compileJava
  :opensearch-sql-plugin:compileJava :integ-test:compileTestJava
  passes locally

Signed-off-by: Kai Huang <ahkcs@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Swiddis Swiddis Swiddis approved these changes

@mengweieric mengweieric mengweieric approved these changes

@ps48 ps48 Awaiting requested review from ps48 ps48 is a code owner

@joshuali925 joshuali925 Awaiting requested review from joshuali925 joshuali925 is a code owner

@dai-chen dai-chen Awaiting requested review from dai-chen dai-chen is a code owner

@vamsimanohar vamsimanohar Awaiting requested review from vamsimanohar vamsimanohar is a code owner

@penghuo penghuo Awaiting requested review from penghuo penghuo is a code owner

@anirudha anirudha Awaiting requested review from anirudha anirudha is a code owner

@acarbonetto acarbonetto Awaiting requested review from acarbonetto acarbonetto is a code owner

@ykmr1224 ykmr1224 Awaiting requested review from ykmr1224 ykmr1224 is a code owner

@LantaoJin LantaoJin Awaiting requested review from LantaoJin LantaoJin is a code owner

@noCharger noCharger Awaiting requested review from noCharger noCharger is a code owner

@qianheng-aws qianheng-aws Awaiting requested review from qianheng-aws qianheng-aws is a code owner

@yuancu yuancu Awaiting requested review from yuancu yuancu is a code owner

@RyanL1997 RyanL1997 Awaiting requested review from RyanL1997 RyanL1997 is a code owner

@songkant-aws songkant-aws Awaiting requested review from songkant-aws songkant-aws is a code owner

Assignees

No one assigned

Labels

bugFix infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. maintenance Improves code quality, but not the product skip-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] SQL CLI integration test flaky failure due to Gradle incremental compilation race condition

3 participants

@ahkcs @Swiddis @mengweieric