bobhan1 commented on code in PR #66609: URL: https://github.com/apache/doris/pull/66609#discussion_r3763226452
########## be/src/storage/transform/partial_update_fill.cpp: ########## @@ -0,0 +1,162 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "storage/transform/partial_update_fill.h" + +#include <algorithm> + +#include "common/cast_set.h" +#include "common/config.h" +#include "core/block/block.h" +#include "storage/iterator/olap_data_convertor.h" +#include "storage/key/row_key_encoder.h" +#include "storage/mow/historical_row_fetcher.h" +#include "storage/mow/key_probe.h" +#include "storage/partial_update_info.h" +#include "storage/rowset/rowset_writer_context.h" +#include "storage/segment/segment_loader.h" +#include "storage/tablet/base_tablet.h" +#include "storage/tablet/tablet_schema.h" +#include "storage/transform/transform_util.h" +#include "util/debug_points.h" + +namespace doris::segment_v2 { + +namespace { + +// Re-adds the merge-on-write sentinel mark to the delete bitmap, when the +// correctness check is on. +void maybe_add_sentinel_mark(TransformExecContext& ctx) { + if (config::enable_merge_on_write_correctness_check) { + ctx.tablet->add_sentinel_mark_to_delete_bitmap(ctx.mow_context->delete_bitmap.get(), + *ctx.mow_context->rowset_ids); + } +} + +// The probe + read-plan loop of the fixed fill. For each row: encode the key +// (with seq suffix when the load provides one), probe the load's rowset +// snapshot, register a brand-new key on a miss, then either flag the row for +// default fill or plan a whole-row historical read. +Status probe_and_plan(TransformExecContext& ctx, RowKeyEncoder& key_encoder, MowKeyProbe& probe, + HistoricalRowFetcher& fetcher, + const std::vector<RowsetSharedPtr>& specified_rowsets, + std::vector<std::unique_ptr<SegmentCacheHandle>>& segment_caches, + const std::vector<IOlapColumnDataAccessor*>& key_columns, + IOlapColumnDataAccessor* seq_column, const signed char* delete_signs, + size_t num_rows, Block* block, std::vector<bool>& use_default_or_null_flag, + bool& has_default_or_nullable) { + const TabletSchema& schema = *ctx.tablet_schema; + PartialUpdateInfo& info = *ctx.partial_update_info; + const bool have_input_seq_column = (seq_column != nullptr); + + use_default_or_null_flag.reserve(num_rows); + for (size_t pos = 0; pos < num_rows; ++pos) { + // Encode without touching the row cache: the writer's key index build + // invalidates every row's cache entry under the same conditions, so the + // erase runs once there, not twice. + // one block == one fresh segment: segment_pos == block row index + std::string key = key_encoder.full_encode_primary_keys(key_columns, pos); + if (have_input_seq_column) { + key_encoder.append_seq_suffix(&key, seq_column, pos); + } + const bool have_delete_sign = (delete_signs != nullptr && delete_signs[pos] != 0); + ProbeOutcome out = DORIS_TRY(probe.probe(key, /*segment_pos=*/pos, have_input_seq_column, + have_delete_sign, specified_rowsets, + segment_caches, ctx.partial_update_stats)); + if (out.result == KeyProbeResult::NOT_FOUND && !have_delete_sign) { + RETURN_IF_ERROR(info.handle_new_key(schema, [&]() -> std::string { + return block->dump_one_line(pos, cast_set<int>(schema.num_key_columns())); + })); + } + has_default_or_nullable |= out.use_default_or_null; + use_default_or_null_flag.emplace_back(out.use_default_or_null); + if (!out.use_default_or_null) { + fetcher.pin_rowset(out.rowset); + fetcher.plan_fixed_read(out.loc, pos); + } + } + CHECK_EQ(use_default_or_null_flag.size(), num_rows); + return Status::OK(); +} + +} // namespace + +Status FixedPartialUpdateFillStage::apply(TransformExecContext& ctx, Block* block) const { + DBUG_EXECUTE_IF("_append_block_with_partial_content.block", DBUG_BLOCK); + + const TabletSchemaSPtr& tablet_schema = ctx.tablet_schema; + const TabletSchema& schema = *tablet_schema; + auto& info = *ctx.partial_update_info; + const size_t num_rows = block->rows(); + + // 1. widen the narrow input to the full schema. The input also keeps any + // generated auto-inc column at the tail, which fill_missing_columns() + // reads from `block` by name. + const auto& update_cids = info.update_cids; + Block full_block = widen_partial_update_block(schema, update_cids, *block); + + // 2. key-only conversion with stage-local encoder + convertor + RowKeyEncoder key_encoder(schema, /*mow=*/true); + // FE forbids partial update on mow tables with cluster keys; everything + // below assumes sort keys == schema keys + DCHECK_EQ(key_encoder.num_sort_key_columns(), schema.num_key_columns()); + OlapBlockDataConvertor convertor; + convertor.resize(schema.num_columns()); + std::vector<IOlapColumnDataAccessor*> key_columns; + RETURN_IF_ERROR(convert_key_columns(convertor, schema, full_block, num_rows, key_columns)); Review Comment: Non-blocking performance concern: convert_key_columns() here, and convert_seq_column() when the input includes the sequence column, convert the data for the historical-row probe. The generic horizontal/vertical writer then converts the completed block again for persistence and primary-key-index construction. The old fixed-partial-update writer path reused its first conversion for both lookup and writing, so this refactor adds a second conversion per fixed partial-update block. This may be noticeable for wide or string composite keys. Could we reuse the converted accessors across the transform/writer boundary, or provide a benchmark showing that the extra conversion is negligible? -- 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]
