airborne12 commented on code in PR #64652:
URL: https://github.com/apache/doris/pull/64652#discussion_r3464421732
##########
be/src/storage/tablet/tablet_meta.cpp:
##########
@@ -543,6 +543,7 @@ void TabletMeta::init_schema_from_thrift(const
TTabletSchema& tablet_schema,
DCHECK_EQ(index.columns.size(), 1);
if (iequal(tcolumn.column_name, index.columns[0])) {
column->set_is_bf_column(true);
+ has_bf_columns = index.index_type ==
TIndexType::type::BLOOMFILTER;
Review Comment:
Blocking: this overwrites the schema-level `has_bf_columns` flag instead of
accumulating it.
A table can legally have an ordinary Bloom Filter on one column and
`NGRAM_BF` on another column. `Index.checkConflict()` only rejects BF and
NGRAM_BF on the same column, not on different columns. With column order like
`k1` using ordinary BF and a later `v1` using `NGRAM_BF`, this line sets
`has_bf_columns` back to `false`. Then `bf_fpp` is not persisted below, and the
segment writer falls back to the default FPP instead of the user-configured
`bloom_filter_fpp`.
Please make this OR-only for ordinary Bloom Filter indexes, for example:
```cpp
if (index.index_type == TIndexType::type::BLOOMFILTER) {
has_bf_columns = true;
}
```
Please also add a BE test for mixed ordinary BF + `NGRAM_BF` with a custom
FPP and the ordinary BF column before the NGRAM_BF column.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
