github-actions[bot] commented on code in PR #66774:
URL: https://github.com/apache/doris/pull/66774#discussion_r3782501187
##########
be/src/storage/transform/block_transform.cpp:
##########
@@ -199,18 +200,21 @@ BlockTransformChain build_transform_chain(const
RowsetWriterContext& context) {
const bool rebuild_row_store = context.write_type ==
DataWriteType::TYPE_DIRECT ||
context.write_type ==
DataWriteType::TYPE_SCHEMA_CHANGE;
if (is_partial_update_load) {
+ // A partial update load is always TYPE_DIRECT, so the row store is
+ // always rebuilt.
if (context.partial_update_info->is_fixed_partial_update()) {
stages.push_back(std::make_shared<FixedPartialUpdateFillStage>());
// The legacy fixed path parsed both provided and missing Variant
- // columns before rebuilding RowStore. A partial update load is
always
- // TYPE_DIRECT, so the row store is always rebuilt.
+ // columns before rebuilding RowStore.
stages.push_back(std::make_shared<VariantParseStage>());
stages.push_back(std::make_shared<RowStoreFillStage>());
- return BlockTransformChain {std::move(stages)};
+ } else {
+
stages.push_back(std::make_shared<FlexiblePartialUpdateFillStage>());
Review Comment:
[P1] Preserve segment numbering when flexible aggregation removes every row
The new stage can turn a nonempty, already-numbered flush into an empty
block. For example, a flexible sequence memtable preserves duplicate keys, and
all rows for segment 0 can be below the historical sequence. Both
segment-writer flushers then return on `row_num == 0` without calling
`segment_collector->add(0, ...)`, but the input-row counter still reports
success. If the next flush writes segment 1,
`BaseBetaRowsetWriter::_num_segment` remains stuck at 0 because its segment set
is missing id 0; with the default metadata check disabled, the rowset is built
with `num_segments == 0`, so readers omit the valid later segment. This PR
newly makes that successful path reachable with
`enable_vertical_segment_writer=false` (the old horizontal writer rejected
flexible updates). Please preserve contiguous numbering when a post-transform
block is empty, and add a two-flush test where the first block shrinks to zero
and the second persists rows in both writer modes.
##########
be/src/storage/segment/segment_writer.cpp:
##########
@@ -334,18 +333,6 @@ Status SegmentWriter::_create_writers(const
TabletSchemaSPtr& tablet_schema,
}
Status SegmentWriter::append_block(const Block* block, size_t row_pos, size_t
num_rows) {
- // Fixed partial update blocks arrive full-width, already filled by the
transform
- // chain; only the flexible mode still needs the vertical writer.
- if (_opts.rowset_ctx->partial_update_info &&
- _opts.rowset_ctx->partial_update_info->is_partial_update() &&
- _opts.write_type == DataWriteType::TYPE_DIRECT &&
- !_opts.rowset_ctx->is_transient_rowset_writer &&
- !_opts.rowset_ctx->partial_update_info->is_fixed_partial_update()) {
- return Status::NotSupported<false>(
- "SegmentWriter doesn't support flexible partial update, please
set "
Review Comment:
[P1] Update the horizontal flexible-update regression expectation
With this rejection removed,
`unique_with_mow_p0/flexible/test_f_segment_writer.groovy` still starts its BE
with `enable_vertical_segment_writer=false` and requires this stream load to
fail with the exact deleted message. The transform now fills and materializes
the valid flexible block before reaching this writer, so the load succeeds and
the existing assertion fails deterministically. Please change the suite to
require success, verify the updated rows and skip bitmap, and regenerate its
expected output so the newly supported horizontal path remains covered end to
end.
--
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]
