Skip to content

Conversation

@liziyan-lzy
Copy link

@liziyan-lzy liziyan-lzy commented Sep 12, 2025

Why I'm doing:

For #62203.

What I'm doing:

This PR adds active planning queries to current_queries view.
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

@CLAassistant
Copy link

CLAassistant commented Sep 12, 2025

CLA assistant check
All committers have signed the CLA.

@murphyatwork
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Oct 28, 2025

rebase

✅ Branch has been successfully rebased

@murphyatwork murphyatwork force-pushed the enhance_current_queries branch from 4cd0496 to 4533a79 Compare October 28, 2025 11:20
@github-actions github-actions bot removed the 3.5 label Oct 28, 2025
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.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

}
} finally {
context.setPlanning(false);
QeProcessorImpl.INSTANCE.unMonitorQuery(context.getExecutionId());
Copy link

Choose a reason for hiding this comment

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

Bug: Query Registration Mismatch Causes Memory Leak

The generateExecPlan method registers a query using registerQuery but calls unMonitorQuery in its finally block. This mismatch means queries are not properly removed from the coordinatorMap, leading to a memory leak as planning queries accumulate.

Fix in Cursor Fix in Web

Copy link
Author

Choose a reason for hiding this comment

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

Since the queryId is taken from the context and removed in the finally block, queries are properly cleaned up, preventing the memory leak issues.

.warehouseName(info.coord.getWarehouseName())
.resourceGroupName(info.coord.getResourceGroupName())
.execState(execState)
.build();
Copy link

Choose a reason for hiding this comment

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

Bug: Builder Misuse: Double .build() Invocation

The itemBuilder incorrectly calls .build() on line 193, discarding the result, and then calls .build() again on line 202. Builders are typically designed for a single .build() invocation, so calling it twice on the same instance can lead to runtime errors or incorrect results.

Fix in Cursor Fix in Web

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

private ExecPlan generateExecPlan() throws Exception {
ExecPlan execPlan = null;
QeProcessorImpl.QueryInfo queryInfo = QeProcessorImpl.QueryInfo.fromPlanningQuery(context, originStmt.originStmt);
QeProcessorImpl.INSTANCE.registerQuery(context.getExecutionId(), queryInfo);
Copy link

Choose a reason for hiding this comment

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

Bug: Double Query Registration Causes Exception

The new code registers a query during planning in generateExecPlan. This causes a double registration issue because the same query is registered again during execution (e.g., in handleQueryStmt) using the same executionId. Since QeProcessorImpl.registerQuery uses putIfAbsent, this results in an AlreadyExistsException.

Fix in Cursor Fix in Web

Copy link
Author

Choose a reason for hiding this comment

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

The query is registered and then removed within the generateExecPlan method. The registration during execution occurs in the subsequent step.

@sonarqubecloud
Copy link

@github-actions
Copy link

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions
Copy link

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@liziyan-lzy liziyan-lzy force-pushed the enhance_current_queries branch from 66c0288 to 6022db3 Compare November 2, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants