morningman commented on code in PR #68959:
URL: https://github.com/apache/doris/pull/68959#discussion_r4246616095
##########
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:
Thanks, the per-object scratch is real, but the importer validates exactly
as branch-4.x does (#66714 uses the same `validate_variant_payload()`), and
master's `insert_encoded_rows()` (used by the JNI reader and for inserted
encoded values) already validates every row this way. The vectors are transient
(freed when each object's check returns) and small (16 bytes per field), and
they replace a full decode and re-encode of every value through
`VariantBatchBuilder`. A reusable, allocator-aware scratch would belong in
`validate_variant_payload()` for all of its callers, so I'll track it as a
separate change rather than special-casing this importer.
--
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]
