hoshinojyunn commented on code in PR #64652:
URL: https://github.com/apache/doris/pull/64652#discussion_r3586625472
##########
fe/fe-core/src/main/java/org/apache/doris/task/CreateReplicaTask.java:
##########
@@ -319,11 +319,14 @@ public TCreateTabletReq toThrift() {
tColumns = (List<TColumn>) tCols;
} else {
tColumns = new ArrayList<>();
+ Set<String> namedBfColumns =
Index.extractBloomFilterColumns(indexes);
for (int i = 0; i < columns.size(); i++) {
Review Comment:
Good catch. The new `namedBfColumns` branch makes the cached `TColumn` list
depend on `indexes`, which is invalid for this pool.
I will not propagate named BF through `bfColumns` or to non-base indexes.
Secondary-index metadata belongs to the base index; forcing a named BF column
into a rollup `bfColumns` would make the rollup materialize a BF without the
named index FPP metadata.
BE already materializes named BF from `TTabletSchema.indexes`: when a
matching `BLOOMFILTER` index is present, `TabletMeta::init_schema_from_thrift`
marks the resulting `ColumnPB` as a BF column. Therefore the FE `TColumn` flag
is redundant for named BF. I will remove
`Index.extractBloomFilterColumns(indexes)` from the cached `CreateReplicaTask`
column conversion and keep the `TColumn` BF flag there only for legacy
`bfColumns`. The base task will send the named index and BE will materialize
it; the rollup task sends no index and will not materialize it. This removes
the new `indexes` dependency from the cache rather than expanding the cache key.
I will add coverage for equal base/rollup schemas in both serialization
orders.
--
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]
