[Enhancement] Make plugin install/uninstall idempotent by s4ch1n · Pull Request #75563 · StarRocks/starrocks

s4ch1n · 2026-06-29T22:17:24Z

Why I'm doing:

Add IF NOT EXISTS / IF EXISTS support for idempotent plugin install/uninstall

#74913

What I'm doing:

Fixes #issue

What type of PR is this:

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 pr needs auto generate documentation
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.1
- 4.0
- 3.5

github-actions · 2026-06-30T17:32:35Z

🌎 Translation Required?

Thanks for your doc contribution! The following languages are missing or outdated for your changes.

🤖 Automated Translations

If you are fluent in any of the missing languages you can add them. If not, a maintainer can generate the translations below.

Maintainer: Check the ones you wish to generate:

`docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md`

zh for docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md (New)
ja for docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md (New)

`docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md`

zh for docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md (New)
ja for docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md (New)

Maintainers: Check boxes and reply with /translate to start.

🕒 Last updated: Fri, 10 Jul 2026 19:25:26 GMT

s4ch1n · 2026-07-06T21:23:35Z

@HangyuanLiu FE Incremental coverage report seems stale, It should be more than 80%, Is there a way to check for staleness in the FE coverage pipeline ? I don't have access to CI logs to check

dirtysalt

LGTM! 🚀

github-actions · 2026-07-10T19:19:18Z

Module-risk briefing for Codex review

This is review focus, not a finding list. Validate against the diff.

High-confidence risks

Module: sql/ast
Changed files: fe/fe-grammar/src/main/antlr/com/starrocks/grammar/StarRocks.g4, fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java, fe/fe-parser/src/main/java/com/starrocks/sql/ast/InstallPluginStmt.java, fe/fe-parser/src/main/java/com/starrocks/sql/ast/UninstallPluginStmt.java
Historical failure pattern: Grammar/parser gap — StarRocks.g4 and AstBuilder repeatedly make implicit parsing choices (default values materialized too early, alternative ordering resolving to the wrong node, tokens that can't be reached from a given rule) that only surface once downstream code needs the distinction.
Review focus: Confirm the new optional (IF NOT EXISTS)? / (IF EXISTS)? clauses can't be ambiguously matched against neighboring tokens/rules in the grammar, and that context.IF() != null in AstBuilder reliably detects the clause across all valid parses (not just the happy path). Also verify the new ifNotExists/ifExists booleans threaded into InstallPluginStmt/UninstallPluginStmt are actually consumed at execution time (PluginMgr) — this module's history shows flags added to AST nodes/constructors that don't get consistently honored downstream.
Evidence: [BugFix] Support ENGINE clause in CREATE TABLE AS SELECT (CTAS) #75771, [BugFix] Allow non-reserved keywords FLOOR/CEIL as column names #75241, [BugFix] support negative value in skew hint #66922, [BugFix] Materialize default VARBINARY length only for explicit OLAP column definitions #70975, [BugFix] Fix ANALYZE TABLE UPDATE HISTOGRAM ON ALL COLUMNS #68290 (from 12 merged bugfixes)

osscm · 2026-07-10T22:16:42Z

+                LOG.info("uninstall plugin[{}] which does not exist", pluginName);
+                return;
+            }
+            throw new DdlException("Plugin " + pluginName + " does not exist");


we can use String.format

osscm · 2026-07-10T22:19:59Z

@s4ch1n @dirtysalt - trying to understand, how does this plugin zip in installed? adding to the plugin folder? and is there any other check SR does before adding to the plugins....

@dirtysalt
It is not about this PR, but in general, from security and safety purpose.
Should we have pre-check about what that plugin is trying to do, or this is SQL to do admin operations, instead of adding it manually to the build?

thanks

s4ch1n · 2026-07-11T18:48:12Z

@osscm I'll try to answer some of the questions.

how does this plugin zip in installed? adding to the plugin folder? and is there any other check SR does before adding to the plugins....

A plugin zip can be installed from either a local path or a remote http/https URL. Once fetched, StarRocks extracts it and moves it into $STARROCKS_HOME/plugins//. More info

Before installing, SR runs a PLUGIN system privilege check, validates required fields in plugin.properties (name, type, version, classname), and confirms the plugin class can actually be loaded - among other checks.

in general, from security and safety purpose. Should we have pre-check about what that plugin is trying to do, or this is SQL to do admin operations, instead of adding it manually to the build?

From a security standpoint though, By default, only root and db_admin roles hold the PLUGIN privilege, so it's already restricted to admin-tier roles. Once loaded, the plugin class runs with the full permissions of the FE JVM process there's no sandboxing or static analysis of intent, but given the lack of sandboxing, admins should treat INSTALL PLUGIN as equivalent to deploying arbitrary code onto the FE, and vet the plugin source themselves beforehand.

github-actions · 2026-07-13T03:54:30Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2026-07-13T03:55:48Z

[FE Incremental Coverage Report]

✅ pass : 8 / 8 (100.00%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/plugin/PluginMgr.java	8	8	100.00%	[]

github-actions · 2026-07-13T03:57:35Z

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

sonarqubecloud · 2026-07-13T04:03:26Z

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dirtysalt

LGTM! 🚀

github-actions Bot added the behavior_changed label Jun 29, 2026

mergify Bot assigned s4ch1n Jun 29, 2026

github-actions Bot requested review from HangyuanLiu and gengjun-git June 29, 2026 22:20

github-actions Bot added the documentation Documentation changes label Jun 30, 2026

github-actions Bot added the docs-maintainer Picked up by the weekly docs triage workflow label Jun 30, 2026

Make plugin install/uninstall idempotent

ae5315a

s4ch1n force-pushed the main-plugin-id branch from cd7a0fb to ae5315a Compare July 4, 2026 20:02

github-actions Bot removed the behavior_changed label Jul 4, 2026

dirtysalt previously approved these changes Jul 9, 2026

View reviewed changes

gengjun-git reviewed Jul 9, 2026

View reviewed changes

Comment thread fe/fe-core/src/main/java/com/starrocks/plugin/PluginMgr.java

Comment thread fe/fe-core/src/main/java/com/starrocks/plugin/PluginMgr.java

dirtysalt assigned dirtysalt and unassigned dirtysalt Jul 9, 2026

Address review comments

e788a44

s4ch1n dismissed dirtysalt’s stale review via e788a44 July 10, 2026 19:17

s4ch1n removed their assignment Jul 10, 2026

s4ch1n requested a review from gengjun-git July 10, 2026 19:59

mergify Bot assigned s4ch1n Jul 10, 2026

osscm reviewed Jul 10, 2026

View reviewed changes

gengjun-git approved these changes Jul 13, 2026

View reviewed changes

dirtysalt enabled auto-merge (squash) July 13, 2026 04:24

dirtysalt approved these changes Jul 13, 2026

View reviewed changes

HangyuanLiu approved these changes Jul 13, 2026

View reviewed changes

Uh oh!

Conversation

s4ch1n commented Jun 29, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌎 Translation Required?

🤖 Automated Translations

docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md

docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md

Uh oh!

s4ch1n commented Jul 6, 2026

Uh oh!

dirtysalt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 10, 2026

Module-risk briefing for Codex review

High-confidence risks

Uh oh!

osscm Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

osscm commented Jul 10, 2026

Uh oh!

s4ch1n commented Jul 11, 2026

Uh oh!

github-actions Bot commented Jul 13, 2026

[Java-Extensions Incremental Coverage Report]

Uh oh!

github-actions Bot commented Jul 13, 2026

[FE Incremental Coverage Report]

file detail

Uh oh!

github-actions Bot commented Jul 13, 2026

[BE Incremental Coverage Report]

Uh oh!

sonarqubecloud Bot commented Jul 13, 2026

Quality Gate passed

Uh oh!

dirtysalt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

s4ch1n commented Jun 29, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Jun 30, 2026 •

edited

Loading

`docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md`

`docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md`