Skip to content

Conversation

HangyuanLiu
Copy link
Contributor

@HangyuanLiu HangyuanLiu commented Sep 29, 2025

Why I'm doing:

What I'm doing:

This pull request refactors and improves the handling of catalog and database statements, especially around the USE, SHOW CREATE DATABASE, and RECOVER DATABASE statements. The changes ensure stricter validation of catalog presence, improve normalization and case-insensitivity, and enhance test coverage for these behaviors.

Key improvements and changes:

1. Catalog Validation and Error Handling

  • Enforces that a catalog must be selected in the context before executing USE, SHOW CREATE DATABASE, and RECOVER DATABASE statements. Throws a semantic exception with a clear error message if not set. (BasicDbStmtAnalyzer.java, AuthorizerStmtVisitor.java) [1] [2] [3]
  • Removes logic that implicitly sets or normalizes the catalog name from statements, instead relying on the current context and erroring if unset. (BasicDbStmtAnalyzer.java)

2. Case Normalization and Parsing

  • Ensures that database names are normalized (case-insensitive) during parsing for SHOW CREATE DATABASE and RECOVER DATABASE statements. (AstBuilder.java) [1] [2]
  • Updates tests to verify that database names are handled case-insensitively. (TableObjectCaseInsensitiveTest.java)

3. Authorization and Privilege Checks

  • Updates privilege checks for SHOW CREATE DATABASE and RECOVER DATABASE to use the catalog from the context and provide accurate error reporting. (AuthorizerStmtVisitor.java) [1] [2]

4. Code Cleanup and Consistency

  • Removes redundant visitor methods and normalizing utilities related to catalog/database name handling. (ShowStmtAnalyzer.java, AstVisitorExtendInterface.java) [1] [2]
  • Adds missing imports and improves static import usage. (AuthorizerStmtVisitor.java, BasicDbStmtAnalyzer.java) [1] [2]

5. Test Coverage Enhancements

  • Adds comprehensive tests for USE statement behavior, including catalog selection, error handling for non-existent catalogs, and execution state validation. (UseDbTest.java)

These changes collectively make catalog/database operations more robust, predictable, and easier to maintain, while improving user feedback and test reliability.

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

@HangyuanLiu HangyuanLiu requested review from a team as code owners September 29, 2025 04:01
@HangyuanLiu HangyuanLiu force-pushed the 0928-ast branch 3 times, most recently from 17fc765 to bcbbd9f Compare September 30, 2025 06:32
Copy link

Copy link

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

[FE Incremental Coverage Report]

pass : 17 / 17 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/sql/analyzer/BasicDbStmtAnalyzer.java 7 7 100.00% []
🔵 com/starrocks/sql/analyzer/AuthorizerStmtVisitor.java 9 9 100.00% []
🔵 com/starrocks/qe/ShowExecutor.java 1 1 100.00% []

@stephen-shelby stephen-shelby merged commit 4485358 into StarRocks:main Oct 14, 2025
114 of 118 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants