github-actions[bot] commented on code in PR #68959:
URL: https://github.com/apache/doris/pull/68959#discussion_r4246251485
##########
be/src/core/column/variant_v2/column_variant_v2.cpp:
##########
@@ -979,42 +994,239 @@ void ColumnVariantV2::insert_encoded_rows( //
NOLINT(readability-function-size)
}
}
- if (_typed || _shredded) {
- // Validate the incoming batch before materializing the destination so
malformed input
- // cannot mutate a typed or segmented Variant column as a side effect.
- ensure_encoded();
+ _append_prevalidated_encoded_data(data);
+}
+
+struct ColumnVariantV2::EncodedRowsAppender::Impl {
+ using MetadataHashHeads =
+ std::unordered_map<size_t, uint32_t, std::hash<size_t>,
std::equal_to<size_t>,
+ CustomStdAllocator<std::pair<const size_t,
uint32_t>>>;
+
+ explicit Impl(ColumnVariantV2& destination) : column(&destination) {}
+
+ ColumnVariantV2* column;
+ MetadataHashHeads metadata_hash_heads;
+ DorisVector<uint32_t> metadata_hash_next;
+ size_t expected_rows = 0;
+ bool initialized = false;
+#ifdef BE_TEST
+ size_t metadata_comparisons = 0;
+#endif
+};
+
+ColumnVariantV2::EncodedRowsAppender::EncodedRowsAppender(ColumnVariantV2&
column)
+ : _impl(std::make_unique<Impl>(column)) {}
+
+ColumnVariantV2::EncodedRowsAppender::~EncodedRowsAppender() = default;
+
+ColumnVariantV2::EncodedRowsAppender::EncodedRowsAppender(EncodedRowsAppender&&)
noexcept = default;
+
+#ifdef BE_TEST
+size_t ColumnVariantV2::EncodedRowsAppender::metadata_comparisons_for_test()
const noexcept {
+ return _impl == nullptr ? 0 : _impl->metadata_comparisons;
+}
+#endif
+
+void ColumnVariantV2::EncodedRowsAppender::append( //
NOLINT(readability-function-size)
+ std::span<const VariantRef> rows) {
+ if (rows.empty()) {
+ return;
}
- DORIS_CHECK(_typed_type == nullptr) << "encoded state cannot retain a
typed data type";
- require_exclusive(_meta_ids, "metadata ids");
- require_exclusive(_values, "values");
- auto& values = *_values;
- auto& metadata_ids = *_meta_ids;
- reserve_rows(values, metadata_ids, data.value_bytes.size, rows);
+ DORIS_CHECK(_impl != nullptr && _impl->column != nullptr)
+ << "Cannot use a moved-from Variant encoded-row appender";
+ ColumnVariantV2& column = *_impl->column;
+
+ using MetadataIdMap =
+ std::unordered_map<std::string_view, uint32_t,
std::hash<std::string_view>,
+ std::equal_to<std::string_view>,
+ CustomStdAllocator<std::pair<const
std::string_view, uint32_t>>>;
+ MetadataIdMap metadata_ids_by_value;
+ DorisVector<VariantMetadataRef> unique_metadatas;
+ DorisVector<uint32_t> source_metadata_ids;
+ DorisVector<StringRef> source_values(rows.size());
+ size_t total_value_bytes = 0;
+ for (size_t row = 0; row < rows.size(); ++row) {
+ const VariantRef value = rows[row];
+ if (value.metadata.data == nullptr && value.metadata.size != 0) {
+ throw Exception(ErrorCode::CORRUPTION,
+ "Variant encoded metadata has a null data pointer
for {} bytes",
+ value.metadata.size);
+ }
+ if (value.value.data == nullptr && value.value.size != 0) {
+ throw Exception(ErrorCode::CORRUPTION,
+ "Variant encoded value has a null data pointer for
{} bytes",
+ value.value.size);
+ }
- if (data.meta_ids.empty()) {
- const VariantMetadataRef metadata = metadata_at(0);
- const uint32_t id = _find_or_insert_metadata({metadata.data,
metadata.size});
- values.insert_many_continuous_binary_data(data.value_bytes.data,
data.value_offsets.data(),
- rows);
- metadata_ids.insert_many_vals(id, rows);
+ const std::string_view metadata_key(
+ value.metadata.data == nullptr ? "" : value.metadata.data,
value.metadata.size);
+ uint32_t source_metadata_id = 0;
+ if (unique_metadatas.empty()) {
+ validate_variant_metadata(value.metadata);
+ unique_metadatas.push_back(value.metadata);
+ } else if (unique_metadatas.size() == 1 &&
metadata_ids_by_value.empty() &&
+ StringRef(unique_metadatas.front().data,
unique_metadatas.front().size) ==
+ StringRef(value.metadata.data,
value.metadata.size)) {
+ // Iceberg files normally share one metadata dictionary across a
batch. Avoid a hash
+ // table and per-row ids until a second distinct dictionary is
actually observed.
+ } else {
+ if (metadata_ids_by_value.empty()) {
+ const VariantMetadataRef first = unique_metadatas.front();
+ metadata_ids_by_value.emplace(
+ std::string_view(first.data == nullptr ? "" :
first.data, first.size), 0);
+ source_metadata_ids.resize(rows.size());
+ }
+ auto metadata_id = metadata_ids_by_value.find(metadata_key);
+ if (metadata_id != metadata_ids_by_value.end()) {
+ source_metadata_id = metadata_id->second;
+ } else {
+ if (unique_metadatas.size() ==
std::numeric_limits<uint32_t>::max()) {
+ throw Exception(
+ ErrorCode::INVALID_ARGUMENT,
+ "Variant encoded metadata dictionary exceeds the
uint32 id limit");
+ }
+ validate_variant_metadata(value.metadata);
+ source_metadata_id =
static_cast<uint32_t>(unique_metadatas.size());
+ unique_metadatas.push_back(value.metadata);
+ metadata_ids_by_value.emplace(metadata_key,
source_metadata_id);
+ }
+ }
+ if (!source_metadata_ids.empty()) {
+ source_metadata_ids[row] = source_metadata_id;
+ }
+ validate_variant_payload(value);
Review Comment:
[P2] Reuse tracked scratch when validating imported objects
Reading an unshredded object through the new importer now calls
`validate_variant_payload()` for every row. Its `require_valid_object()` in
`variant_field.cpp` constructs a fresh `std::vector<ObjectValueSpan>` and
reserves one entry per field; nested objects retain their ancestors' vectors
until recursion returns. Thus a 4096-row batch of nonempty objects incurs at
least 4096 scratch allocations, and a wide object allocates 16 bytes per field
on 64-bit BE outside Doris memory tracking: ordinary malloc only delegates to
jemalloc, whereas `DorisVector` uses the checked/tracked `Allocator` path. The
previous reader's `VariantBatchBuilder::validate_import_object()` reused a
tracked `DorisVector` across rows. Please supply reusable allocator-aware
validation scratch for this path while retaining the corruption checks, and
cover object/nested payload allocation accounting; the new scalar import tests
do not exercise this regression.
--
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]
