Skip to content

Conversation

HangyuanLiu
Copy link
Contributor

@HangyuanLiu HangyuanLiu commented Aug 29, 2025

Why I'm doing:

What I'm doing:

This pull request refactors index-related utility code in the schema change handler, mainly by moving Bloom filter and inverted index validation logic out of the analysis package and into the sql.analyzer package. It also updates how indexes are created and validated during schema changes, improving code organization and clarity.

Refactoring and Code Organization

  • Removed BloomFilterIndexUtil.java and InvertedIndexUtil.java from the analysis package, consolidating their logic into IndexAnalyzer in the sql.analyzer package for better separation of concerns and maintainability. [1] [2] [3] [4]

Index Creation and Validation Improvements

  • Updated index creation logic in SchemaChangeHandler.java to construct Index objects directly from IndexDef, and to assign index IDs only for compatible index types on OLAP tables, ensuring correct index metadata.
  • Changed Bloom filter and ngram Bloom filter analysis calls to use IndexAnalyzer instead of the removed utility class, centralizing validation logic.
  • Updated column validation for indexes to use IndexAnalyzer.checkColumn for consistent validation across index types.

Minor Cleanups

  • Removed the unused tableRefToSql() method from TableRef.java and deleted the now-unused ToSqlUtils.java utility class, cleaning up obsolete code. [1] [2]

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 August 29, 2025 06:21
Copy link

@alvin-celerdata
Copy link
Contributor

@cursor review

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!


Copy link

github-actions bot commented Sep 1, 2025

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

github-actions bot commented Sep 1, 2025

[FE Incremental Coverage Report]

pass : 212 / 222 (95.50%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/sql/analyzer/IndexAnalyzer.java 190 200 95.00% [64, 127, 168, 210, 211, 222, 242, 293, 354, 355]
🔵 com/starrocks/server/OlapTableFactory.java 1 1 100.00% []
🔵 com/starrocks/alter/SchemaChangeHandler.java 13 13 100.00% []
🔵 com/starrocks/sql/analyzer/MaterializedViewAnalyzer.java 3 3 100.00% []
🔵 com/starrocks/sql/analyzer/CreateTableAnalyzer.java 3 3 100.00% []
🔵 com/starrocks/sql/analyzer/AlterTableClauseAnalyzer.java 1 1 100.00% []
🔵 com/starrocks/common/util/PropertyAnalyzer.java 1 1 100.00% []

Copy link

github-actions bot commented Sep 1, 2025

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@HangyuanLiu HangyuanLiu merged commit 4556a91 into StarRocks:main Sep 1, 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