[feat](fe) Support named bloom filter indexes in DDL and schema change#64652
[feat](fe) Support named bloom filter indexes in DDL and schema change#64652hoshinojyunn wants to merge 1 commit into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29361 ms |
TPC-DS: Total hot run time: 175787 ms |
ClickBench: Total hot run time: 25.23 s |
39129a2 to
0e0cf67
Compare
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
FE UT Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
80d0150 to
1be7525
Compare
|
run buildall |
|
/review |
There was a problem hiding this comment.
I found three issues that should be fixed before merging.
Checkpoint conclusions:
- Task goal and implementation: the named BLOOMFILTER syntax is wired through parser/catalog/schema-change paths, but materialization is incomplete for non-base materialized indexes.
- Tests: good coverage for parser, FE validation, BE tablet metadata, and regression DDL; missing rollup/schema-change materialization and backup/restore signature coverage for named-only bloom filters.
- Compatibility and persistence: named-only
bloom_filter_fppis not reflected in the table signature used by restore schema comparison. - Parallel paths: local/cloud schema-change and create-table/add-partition paths need the same effective bloom-filter-column handling.
- Performance/concurrency/lifecycle: no separate blocking issue found in locking, lifecycle, or performance-sensitive code.
- User focus: no additional user-provided focus points were present.
| tColumns = (List<TColumn>) tCols; | ||
| } else { | ||
| tColumns = new ArrayList<>(); | ||
| Set<String> namedBfColumns = Index.extractBloomFilterColumns(indexes); |
There was a problem hiding this comment.
Named bloom filter columns are derived only from indexes, but several existing non-base-index callers still pass null here. For example, SchemaChangeJobV2.createShadowIndexReplica() only passes indexes for the base index, while addShadowIndexToCatalog() writes the new index list into every changed shadow index meta when indexChange is true. ALTER TABLE ... ADD INDEX bf(v1) USING BLOOMFILTER on a table with a rollup containing v1 will therefore create the rollup shadow tablets without is_bloom_filter_column or bloom_filter_fpp, but the catalog then advertises the bloom filter index for that rollup. The same base-only indexes pattern exists in the cloud schema-change path and initial partition creation. Please pass the effective named bloom-filter column set to every materialized index schema that contains those columns, or fold named bloom-filter columns into the bfColumns argument, and add a rollup/schema-change test.
There was a problem hiding this comment.
There is a real mismatch here, but I am treating it as an existing semantic boundary rather than something this PR should partially fix.
Current behavior is:
- non-base materialized indexes (for example rollup/MV shadow replicas, and the corresponding create-table / add-partition non-base paths) still do not receive named secondary-index metadata in replica/create-tablet tasks;
- legacy BF columns are still propagated through
bfColumnsfrom table properties; - FE display/meta now shows the effective BF columns from both legacy table properties and named
BLOOMFILTERindexes.
So today named BF on non-base indexes is indeed not fully aligned between physical materialization and FE display. I do not want to "fix" this in this PR by only passing named BF indexes into rollup schema-change replica tasks, because that would make rollup behavior differ before vs after schema change and would deepen the split between the legacy bloom_filter_columns path and the named-index path.
The planned direction is to refactor bloom filter metadata as a whole, gradually deprecate/remove the legacy table-property mechanism for BF columns, and then move BF fully onto named BLOOMFILTER indexes. Once that migration is ready, non-base materialization and display semantics can be unified cleanly.
So for this PR I am keeping the scope to:
- named
BLOOMFILTERDDL support - index-level
bloom_filter_fpp - FE/BE materialization for the currently supported paths
- compatibility with legacy
bloom_filter_columns - FE/BE UT and regression coverage in that scope
I will note this limitation explicitly in the PR description.
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 30288 ms |
TPC-DS: Total hot run time: 173769 ms |
ClickBench: Total hot run time: 25.13 s |
FE Regression Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 29989 ms |
TPC-DS: Total hot run time: 173930 ms |
ClickBench: Total hot run time: 25.35 s |
FE UT Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Found four correctness issues in the named BLOOMFILTER implementation: invalid index-level FPP errors are silently converted, local create-replica column caching can reuse named-BF flags from the wrong context, root VARIANT named BLOOMFILTER is accepted but physically suppressed before writer creation, and mixed light-index ALTER can publish BLOOMFILTER metadata without full tablet-schema materialization.
Validation was static only: this checkout is missing .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc, and the review prompt forbids source edits/builds. Also, Round 3 produced the last two accepted findings at the configured three-round cap, so convergence could not be rerun after the final comment-set update.
Assumption / semantic boundary:
A named bloom filter index and a property-managed bloom filter index cannot be
defined on the same column. The two metadata forms are managed separately, and
users must use `DROP INDEX` for named bloom filter indexes and
`ALTER TABLE SET("bloom_filter_columns" = ...)` for property-managed bloom
filter indexes.
Problem Summary:
Previously Doris only supported bloom filter indexes through table properties such as
`"bloom_filter_columns"` and `"bloom_filter_fpp"`. That made bloom filter indexes
behave differently from other index types and prevented users from managing them with
named `INDEX` / `CREATE INDEX` / `DROP INDEX` syntax.
This change adds named `USING BLOOMFILTER` syntax in the Nereids parser and FE DDL
pipeline, keeps the existing table-property bloom filter behavior for legacy metadata,
and enforces that legacy bloom filter columns and named bloom filter indexes cannot be
defined on the same column. The schema change path now distinguishes legacy bloom
filter management from named bloom filter management. Named BF indexes using index-level
`bloom_filter_fpp` properties while legacy bloom filter columns use table-level
`bloom_filter_fpp` properties.
The patch also fixes bloom filter materialization on shadow columns and ensures tablet
metadata marks named bloom filter columns correctly on the BE side. FE unit tests and
bloom filter regression tests are added to cover parser analysis, semantic validation,
schema change checks, FE->BE task generation, and named bloom filter DDL behavior.
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29683 ms |
TPC-DS: Total hot run time: 178326 ms |
ClickBench: Total hot run time: 25.17 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Doris historically manages Bloom Filter columns through table properties:
bloom_filter_columnsbloom_filter_fppThat creates several gaps:
bloom_filter_fppinstead of relying on the legacy table-level property model.This PR adds named
USING BLOOMFILTERsupport while keeping compatibility with legacybloom_filter_columns. A named Bloom Filter index and a legacy BF column cannot be defined on the same column, but they can coexist on different columns.Assumption / semantic boundary:
A named bloom filter index and a property-managed bloom filter index cannot be
defined on the same column. The two metadata forms are managed separately, and
users must use DROP INDEX for named bloom filter indexes and
ALTER TABLE SET("bloom_filter_columns" = ...) for property-managed bloom
filter indexes.
Main changes
Parser and analysis
BLOOMFILTERsyntax in Nereids for inline index definitions,CREATE INDEX, andALTER TABLE ... ADD INDEX.PROPERTIES("bloom_filter_fpp" = "...")on named Bloom Filter indexes.FE metadata and DDL semantics
BLOOMFILTERindexes in table/index metadata and keep legacybloom_filter_columnscompatibility.bloom_filter_fppdifferences are visible in schema comparison.CREATE INDEX/ADD INDEX/DROP INDEXALTER TABLE SET ("bloom_filter_columns" = ...)FE -> BE materialization
BLOOMFILTERindexes in thrift tablet schema creation, protobuf/cloud tablet meta creation, and create-replica paths.bloom_filter_fppbloom_filter_fppDisplay / observability
SHOW PROC/DESCBF presentation to display both legacy BF columns and namedBLOOMFILTERcolumns.Tests
BLOOMFILTERsyntax and semantics.SHOW PROC, andDESC.CREATE INDEX/DROP INDEX, named + legacy mixed cases,SHOW INDEX/DESC/SHOW PROC, rollup display cases, and BF FPP behavior / compatibility checks.http_logsdataset:BLOOMFILTERindex on the same logical columnset enable_profile = trueandset profile_level = 2above can be summarized:
~/and compare Bloom Filter related profile fields / parametersExamples
Current scope / known limitation
This PR keeps the current non-base materialized-index boundary unchanged.
During schema change / create-partition paths, non-base indexes still do not receive named secondary-index metadata in replica tasks, while FE display shows effective BF columns from the merged legacy + named metadata view. Full alignment of non-base BF materialization and display will be handled in a follow-up Bloom Filter refactor that migrates BF management fully to named
BLOOMFILTERindexes and gradually deprecates the legacybloom_filter_columnspath.Release note
Doris now supports named
USING BLOOMFILTERindexes with optional index-levelbloom_filter_fpp, while remaining compatible with legacy table-property Bloom Filter columns. Legacy BF columns and named BF indexes cannot be defined on the same column.Check List (For Author)
Check List (For Reviewer who merge this PR)