github-actions[bot] commented on code in PR #64905:
URL: https://github.com/apache/doris/pull/64905#discussion_r3487902650
##########
be/src/core/column/column_array.h:
##########
@@ -216,14 +215,9 @@ class ColumnArray final : public COWHelper<IColumn,
ColumnArray> {
return get_offsets()[i] - get_offsets()[i - 1];
}
- void for_each_subcolumn(MutableColumnCallback callback) override {
- IColumn::WrappedPtr
offsets_column(std::move(static_cast<ColumnOffsets::Ptr&>(offsets)));
- Defer defer([&] {
- static_cast<ColumnOffsets::Ptr&>(offsets) =
- cast_to_column<ColumnOffsets>(static_cast<const
IColumn::Ptr&>(offsets_column));
- });
- callback(offsets_column);
- callback(data);
+ void mutate_subcolumns() override {
Review Comment:
This new hook still needs a shared-detach test. The added array COW test
builds the array from moved mutable children and then calls
`IColumn::mutate(std::move(array))`, so it only exercises the exclusive fast
path; it would not catch a regression where the by-value/shared
`IColumn::mutate(array)` path forgets to detach either the nested data or the
typed offsets. Please add a small test analogous to the nullable/map shared
tests that keeps aliases to both child columns, calls `IColumn::mutate(array)`
without moving the source owner, and verifies the result detached both children
while the aliases remain unchanged. Since this PR also adds
`mutate_subcolumns()` hooks for `ColumnConst` and `ColumnStruct`, small direct
mutate tests for those hooks would keep the COW refactor covered across all
changed composite columns.
--
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]
