Skip to content

[BugFix] Fix usage and record for changing warehouse in inline comment (backport #66677) #66734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
wanpengfei-git merged 1 commit into from
Dec 15, 2025
Merged

[BugFix] Fix usage and record for changing warehouse in inline comment (backport #66677) #66734

wanpengfei-git merged 1 commit into from
Dec 15, 2025

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Dec 15, 2025 •
edited by cursor bot
Loading

Why I'm doing:

For the inline comment /*+SET_VAR(warehouse=wh2)*/, both the warehouse recorded in Query Detail and the warehouse actually used at runtime are incorrect.

  1. Warehouse recorded in Query Detail

    • Problem: addRunningQueryDetail is called first, and only later does processQueryScopeHint parse the warehouse hint.
    • Fix: In addRunningQueryDetail, call processQueryScopeSetVarHint to update the session variables, and restore the original session variables at the end of addRunningQueryDetail.

  2. Warehouse actually used by the query

    • Problem: The resource group for a query is determined by ConnectContext.computeResource. This is a cache that is only reset when a query finishes. At query start, computeResource has already cached the current session variable’s warehouse, so a later inline SET_VAR hint won’t refresh the cached computeResource.
    • Fix: When SET_VAR updates session variables via setSystemVariable, also update computeResource, similar to how SetExecutor handles set warehouse = 'wh2'.

What I'm doing:

Fixes #issue

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.0
    • 3.5
    • 3.4
    • 3.3

Note

Ensure changing warehouse via inline SET_VAR or session variable resets compute resource and updates query detail, coordinator, and audit log.

  • QE execution flow:
    • After processQueryScopeHint, if the current warehouse changed, update QueryDetail.warehouse and re-add running query detail.
    • Audit builder continues to record current warehouse and CN group.
  • Session variable management:
    • VariableMgr.setSystemVariable(...) now accepts ConnectContext; when setting warehouse, invoke handleSetWarehouse to reset/ensure compute resource accordingly.
    • New handleSetWarehouse resets compute resource when warehouse changes; otherwise re-acquires for availability.
    • Kept backward-compatible overload of setSystemVariable without context.
  • Tests:
    • Add end-to-end tests validating warehouse selection via inline hint and via session set warehouse, asserting QueryDetail.warehouse, coordinator's compute resource, and audit event warehouse.
    • Add test ensuring compute resource reflects warehouse changes via SET warehouse in shared-data mode.

Written by Cursor Bugbot for commit 2b0ee90. This will update automatically on new commits. Configure here.

This is an automatic backport of pull request #66677 done by [Mergify](https://mergify.com).

Note

Processes inline SET_VAR hints prior to adding running query detail (with session var backup/restore), plus minor refactors in hint handling.

  • QE (StmtExecutor):
    • New logic: Add processQueryScopeSetVarHint() to apply query-scope SET_VAR hints by cloning and updating SessionVariable.
    • Query detail recording: In addRunningQueryDetail, call processQueryScopeSetVarHint() before building QueryDetail, then restore original session variables in finally.
    • Hint handling refactor:
      • Use new ConcurrentHashMap<>(context.getUserVariables()) when cloning user variables.
      • Remove @VisibleForTesting from processQueryScopeHint.

Written by Cursor Bugbot for commit 117bcbc. This will update automatically on new commits. Configure here.

@mergify mergify bot added the conflicts label Dec 15, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Dec 15, 2025

Cherry-pick of 04bebb8 has failed:

On branch mergify/bp/branch-3.5/pr-66677
Your branch is up to date with 'origin/branch-3.5'.

You are currently cherry-picking commit 04bebb8e81.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java
	both modified:   fe/fe-core/src/main/java/com/starrocks/qe/VariableMgr.java
	both modified:   fe/fe-core/src/test/java/com/starrocks/qe/ConnectProcessorTest.java
	both modified:   fe/fe-core/src/test/java/com/starrocks/sql/plan/SetVarTest.java

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot mentioned this pull request Dec 15, 2025
Merged
23 tasks
@wanpengfei-git wanpengfei-git enabled auto-merge (squash) December 15, 2025 08:43
@mergify mergify bot closed this Dec 15, 2025
auto-merge was automatically disabled December 15, 2025 08:44

Pull request was closed

@mergify
Copy link
Contributor Author

mergify bot commented Dec 15, 2025

@mergify[bot]: Backport conflict, please reslove the conflict and resubmit the pr

@mergify
Copy link
Contributor Author

mergify bot commented Dec 15, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for 117bcbc.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on branch-3.5 Retries 🔍 CI Insights 📄 Logs
CI PIPELINE - BRANCH FE UT Base branch is broken, but retries were needed. Could be early signs of flakiness 👀 Broken 3 View View
PR CHECKER automerge-check Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@ZiheLiu ZiheLiu reopened this Dec 15, 2025
@wanpengfei-git wanpengfei-git enabled auto-merge (squash) December 15, 2025 08:49
@ZiheLiu
fix
117bcbc 
Signed-off-by: zihe.liu <ziheliu1024@gmail.com>
@ZiheLiu ZiheLiu force-pushed the mergify/bp/branch-3.5/pr-66677 branch from c2bba20 to 117bcbc Compare December 15, 2025 09:10
@alvin-celerdata
Copy link
Contributor

alvin-celerdata commented Dec 15, 2025

@cursor review

cursor[bot]
cursor bot reviewed Dec 15, 2025
View reviewed changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!

@wanpengfei-git wanpengfei-git merged commit 6c60151 into branch-3.5 Dec 15, 2025
72 of 75 checks passed
@wanpengfei-git wanpengfei-git deleted the mergify/bp/branch-3.5/pr-66677 branch December 15, 2025 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@alvin-celerdata alvin-celerdata alvin-celerdata approved these changes

Assignees

@ZiheLiu ZiheLiu

Labels

automerge conflicts version:3.5.11

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alvin-celerdata @ZiheLiu @wanpengfei-git