Copilot commented on code in PR #64197:
URL: https://github.com/apache/doris/pull/64197#discussion_r3370653051
##########
be/src/storage/segment/variant/variant_column_writer_impl.cpp:
##########
@@ -555,6 +636,266 @@ Status append_sparse_converted_column(const TabletColumn&
tablet_column, ColumnW
converter->clear_source_content(cid);
return Status::OK();
}
+
+bool has_doc_value_data(const ColumnVariant& variant) {
+ if (variant.size() == 0) {
+ return false;
+ }
+ const auto& offsets = variant.serialized_doc_value_column_offsets();
+ return !offsets.empty() && offsets[variant.size() - 1] > 0;
+}
+
+// The variant root column is always written by _process_root_column(). The
plan below only decides
+// how to write non-root variant data: extracted subcolumns, sparse columns,
or doc-value buckets.
+// Used when extracted columns already own all non-root variant data.
+struct ExtractedColumnsOwnDataPlan {};
+
+// Used when JSON parse already expanded paths into ColumnVariant subcolumns.
+struct ParseTimeSubcolumnsWritePlan {
+ // True when sparse columns are generated from the parse tree after top-N
selection.
+ bool write_sparse_columns = false;
+};
+
+// Used when plain non-doc VARIANT arrives as temporary doc-value KV staging.
+struct DocValueStagingWritePlan {};
+
+// Used by persistent doc mode; non-root data is written to doc-value bucket
columns.
+struct PersistentDocValueWritePlan {};
+
+using VariantNonRootWritePlan =
+ std::variant<ExtractedColumnsOwnDataPlan, ParseTimeSubcolumnsWritePlan,
+ DocValueStagingWritePlan, PersistentDocValueWritePlan>;
+
+template <typename... Visitors>
+struct Overloaded : Visitors... {
+ using Visitors::operator()...;
+};
+template <typename... Visitors>
+Overloaded(Visitors...) -> Overloaded<Visitors...>;
+
+VariantNonRootWritePlan build_variant_non_root_write_plan(const TabletColumn&
tablet_column,
+ const ColumnVariant&
variant,
+ bool
has_extracted_columns) {
+ if (has_extracted_columns) {
+ return ExtractedColumnsOwnDataPlan {};
+ }
+
+ // Plain non-doc VARIANT may arrive as doc-value KV staging from storage
parse. The staging data
+ // is internal to this root writer and is converted into materialized
subcolumns plus sparse
+ // columns.
+ if (!tablet_column.variant_enable_doc_mode() &&
!tablet_column.variant_enable_nested_group() &&
+ has_doc_value_data(variant)) {
+ return DocValueStagingWritePlan {};
+ }
+
+ if (tablet_column.variant_enable_doc_mode()) {
+ return PersistentDocValueWritePlan {};
+ }
+
+ return ParseTimeSubcolumnsWritePlan {
+ .write_sparse_columns =
+
variant_util::should_write_variant_binary_columns(tablet_column)};
+}
+
+Status collect_typed_subcolumn_info_from_parse_tree(
+ const ColumnVariant& variant, const TabletColumn& tablet_column,
+ const TabletSchema& tablet_schema,
+ std::unordered_map<std::string, TabletSchema::SubColumnInfo>*
subcolumns_info) {
+ for (const auto& entry :
variant_util::get_sorted_subcolumns(variant.get_subcolumns())) {
+ if (entry->path.empty()) {
+ // already handled
+ continue;
+ }
+ // Not supported nested path to generate sub column info, currently
+ if (entry->path.has_nested_part()) {
+ continue;
+ }
+ TabletSchema::SubColumnInfo sub_column_info;
+ if (variant_util::generate_sub_column_info(tablet_schema,
tablet_column.unique_id(),
+ entry->path.get_path(),
&sub_column_info)) {
+ subcolumns_info->emplace(entry->path.get_path(),
std::move(sub_column_info));
+ }
+ }
+ return Status::OK();
+}
+
+Status prepare_parse_time_subcolumns_for_write(
+ ColumnVariant* variant, const TabletColumn& tablet_column,
+ const TabletSchema& tablet_schema,
+ std::unordered_map<std::string, TabletSchema::SubColumnInfo>*
subcolumns_info) {
+ // Temporary doc-value staging has no parse-time subcolumns, so only
ParseTimeSubcolumnsWritePlan
+ // reaches this helper. Parse-time paths still need typed-path storage
conversion before their
+ // writers are created.
+ RETURN_IF_ERROR(collect_typed_subcolumn_info_from_parse_tree(*variant,
tablet_column,
+
tablet_schema, subcolumns_info));
+
RETURN_IF_ERROR(variant->convert_typed_path_to_storage_type(*subcolumns_info));
+ return Status::OK();
+}
+
+Status prepare_non_root_write_plan_before_write(
+ const VariantNonRootWritePlan& non_root_write_plan, ColumnVariant*
variant,
+ const TabletColumn& tablet_column, const TabletSchema& tablet_schema,
+ std::unordered_map<std::string, TabletSchema::SubColumnInfo>*
subcolumns_info) {
+ if
(std::holds_alternative<ParseTimeSubcolumnsWritePlan>(non_root_write_plan)) {
+ RETURN_IF_ERROR(prepare_parse_time_subcolumns_for_write(variant,
tablet_column,
+ tablet_schema,
subcolumns_info));
+ }
+ return Status::OK();
+}
+
+Status prepare_sparse_columns_from_parse_tree(
+ const VariantNonRootWritePlan& non_root_write_plan, ColumnVariant*
variant,
+ const TabletColumn& tablet_column,
+ const std::unordered_map<std::string, TabletSchema::SubColumnInfo>&
subcolumns_info) {
+ const auto* parse_time_plan =
std::get_if<ParseTimeSubcolumnsWritePlan>(&non_root_write_plan);
+ if (parse_time_plan == nullptr || !parse_time_plan->write_sparse_columns) {
+ return Status::OK();
+ }
+ return variant->pick_subcolumns_to_sparse_column(
+ subcolumns_info,
tablet_column.variant_enable_typed_paths_to_sparse());
+}
+
+struct RegularVariantDocValuePlan {
+ DocValuePathStats stats;
+ DocValuePathSet materialized_paths;
+};
+
+size_t dotted_path_depth(StringRef path) {
+ return static_cast<size_t>(std::count(path.data, path.data + path.size,
'.'));
+}
+
+Status build_regular_variant_doc_value_plan(
+ const ColumnVariant& variant, const TabletColumn& parent_column,
+ const TabletSchema& tablet_schema,
+ std::unordered_map<std::string, TabletSchema::SubColumnInfo>*
subcolumns_info,
+ RegularVariantDocValuePlan* plan) {
+ build_doc_value_stats(variant, &plan->stats);
+ if (plan->stats.empty()) {
+ return Status::OK();
+ }
+
+ struct Candidate {
+ StringRef path;
+ uint32_t non_null_count = 0;
+ };
+ std::vector<Candidate> dynamic_candidates;
+ dynamic_candidates.reserve(plan->stats.size());
+ const bool materialize_all_dynamic_paths =
parent_column.variant_max_subcolumns_count() == 0;
+
+ for (const auto& [path_ref, non_null_count] : plan->stats) {
+ if (path_ref.size == 0 || non_null_count == 0) {
+ continue;
+ }
+ std::string path(path_ref.data, path_ref.size);
+ TabletSchema::SubColumnInfo sub_column_info;
+ const bool is_typed_path = variant_util::generate_sub_column_info(
+ tablet_schema, parent_column.unique_id(), path,
&sub_column_info);
+ if (is_typed_path) {
+ subcolumns_info->emplace(path, std::move(sub_column_info));
+ }
+ if (is_typed_path &&
!parent_column.variant_enable_typed_paths_to_sparse()) {
+ plan->materialized_paths.emplace(path_ref);
+ continue;
+ }
+ dynamic_candidates.push_back({path_ref, non_null_count});
+ }
+
+ std::sort(dynamic_candidates.begin(), dynamic_candidates.end(),
+ [](const Candidate& lhs, const Candidate& rhs) {
+ if (lhs.non_null_count != rhs.non_null_count) {
+ return lhs.non_null_count > rhs.non_null_count;
+ }
+ const auto lhs_depth = dotted_path_depth(lhs.path);
+ const auto rhs_depth = dotted_path_depth(rhs.path);
+ if (lhs_depth != rhs_depth) {
+ return lhs_depth > rhs_depth;
+ }
+ return std::string_view(lhs.path.data, lhs.path.size) >
+ std::string_view(rhs.path.data, rhs.path.size);
+ });
+
+ const size_t dynamic_limit =
+ materialize_all_dynamic_paths
+ ? dynamic_candidates.size()
+ :
static_cast<size_t>(parent_column.variant_max_subcolumns_count());
+ for (size_t i = 0; i < std::min(dynamic_limit, dynamic_candidates.size());
++i) {
+ plan->materialized_paths.emplace(dynamic_candidates[i].path);
+ }
+ return Status::OK();
+}
+
+Status append_typed_doc_value_to_sparse_column(const ColumnString* doc_values,
size_t doc_value_pos,
+ std::string_view path,
+ const DataTypePtr& storage_type,
+ ColumnString* sparse_keys,
+ ColumnString* sparse_values) {
+ ColumnVariant::Subcolumn subcolumn(0, true, false);
+ subcolumn.deserialize_from_binary_column(doc_values, doc_value_pos);
+ subcolumn.finalize(ColumnVariant::FinalizeMode::WRITE_MODE);
+
+ ColumnPtr current_column = subcolumn.get_finalized_column_ptr()->get_ptr();
+ DataTypePtr current_type = subcolumn.get_least_common_type();
+ if (!storage_type->equals(*current_type)) {
+ RETURN_IF_ERROR(variant_util::cast_column({current_column,
current_type, ""}, storage_type,
+ ¤t_column));
+ }
+
+ DataTypePtr sparse_type = storage_type;
+ if (!current_column->is_nullable()) {
+ current_column = make_nullable(current_column);
+ sparse_type = make_nullable(storage_type);
+ }
+
+ auto mutable_column = IColumn::mutate(std::move(current_column));
+ ColumnVariant::Subcolumn typed_subcolumn(std::move(mutable_column),
sparse_type, true, false);
+ typed_subcolumn.serialize_to_binary_column(sparse_keys, path,
sparse_values, 0);
+ return Status::OK();
+}
+
+Status build_sparse_column_from_doc_values(
+ const ColumnVariant& variant, const DocValuePathSet&
materialized_paths,
+ const std::unordered_map<std::string, TabletSchema::SubColumnInfo>&
typed_paths,
+ size_t num_rows, MutableColumnPtr* result) {
+ auto sparse_column = ColumnVariant::create_binary_column_fn();
+ auto& sparse_map = assert_cast<ColumnMap&>(*sparse_column);
+ auto& sparse_keys = assert_cast<ColumnString&>(sparse_map.get_keys());
+ auto& sparse_values = assert_cast<ColumnString&>(sparse_map.get_values());
+ auto& sparse_offsets = sparse_map.get_offsets();
+ sparse_offsets.reserve(num_rows);
+
+ std::unordered_map<std::string_view, DataTypePtr> typed_storage_types;
+ typed_storage_types.reserve(typed_paths.size());
+ for (const auto& [path, subcolumn_info] : typed_paths) {
+ typed_storage_types.emplace(
+ std::string_view(path.data(), path.size()),
+
DataTypeFactory::instance().create_data_type(subcolumn_info.column));
+ }
+
+ const auto [doc_keys, doc_values] =
variant.get_doc_value_data_paths_and_values();
+ const auto& doc_offsets = variant.serialized_doc_value_column_offsets();
+ for (size_t row = 0; row < num_rows; ++row) {
+ const size_t start = doc_offsets[row - 1];
Review Comment:
`row` starts at 0, so `doc_offsets[row - 1]` underflows on the first
iteration and will read out of bounds. Use 0 as the start offset for the first
row when scanning the doc-value entries.
##########
be/src/storage/segment/variant/variant_column_writer_impl.cpp:
##########
@@ -242,6 +267,9 @@ void build_sparse_subcolumns(const ColumnVariant& variant,
const DocValuePathSta
const size_t end = column_offsets[row];
for (size_t i = start; i < end; ++i) {
Review Comment:
In the per-row doc-value iteration, the start offset is computed with
`column_offsets[row - 1]` just above this line. When `row == 0` that underflows
and can read out of bounds, leading to undefined behavior. Use `0` as the start
offset for the first row.
##########
be/src/storage/segment/variant/variant_column_writer_impl.cpp:
##########
@@ -203,8 +205,30 @@ struct SubcolumnWritePlan {
DocValuePathStats stats;
};
+enum class DocValueMaterializationMode {
+ // Materialize every doc-value path into a full column.
+ DenseAllPaths,
+ // Materialize every doc-value path as values plus row ids; gaps are
filled during write.
+ RowIdAllPaths,
+ // Materialize only selected doc-value paths as values plus row ids. The
caller keeps
+ // unselected paths in doc-value or sparse columns.
+ RowIdSelectedPaths,
+};
+
+struct DocValueMaterializationOptions {
+ int64_t min_rows = 0;
+ // Full doc-value path statistics already computed by the caller. It is an
optimization only:
+ // stats stay scoped to all doc-value paths, never just selected
materialized paths.
+ const DocValuePathStats* precomputed_stats = nullptr;
+ // Optional materialized path filter. Used by plain non-doc staging after
choosing which paths
+ // become real subcolumns; the remaining doc-value items are emitted to
sparse columns.
+ const DocValuePathSet* selected_paths = nullptr;
+};
+
constexpr size_t kInitialDocPathReserve = 8192;
+void release_processed_subcolumn_write_entry(SubcolumnWriteEntry* entry);
+
// Build per-path non-null counts from the serialized doc-value representation.
void build_doc_value_stats(const ColumnVariant& variant, DocValuePathStats*
stats) {
auto [column_key, column_value] =
variant.get_doc_value_data_paths_and_values();
Review Comment:
The row loop in this helper computes the per-row slice using
`column_offsets[row - 1]`. When `row == 0` that underflows and can read out of
bounds, causing undefined behavior. Use 0 as the start offset for the first row.
--
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]
