This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 00537142bf8 branch-4.2: [improvement](variant) Reduce Variant array
shredding CPU during import #67983 (#68296)
00537142bf8 is described below
commit 00537142bf8e9aedfcaec90eff6792c997eab9b6
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Sep 21 17:07:53 2026 +0800
branch-4.2: [improvement](variant) Reduce Variant array shredding CPU
during import #67983 (#68296)
Cherry-picked from #67983
Co-authored-by: lihangyu <[email protected]>
Co-authored-by: Claude <[email protected]>
---
.../segment/variant/v2/variant_path_builder.cpp | 58 ++++++++++++++------
.../variant/variant_column_writer_reader_test.cpp | 64 ++++++++++++++++++++++
2 files changed, 104 insertions(+), 18 deletions(-)
diff --git a/be/src/storage/segment/variant/v2/variant_path_builder.cpp
b/be/src/storage/segment/variant/v2/variant_path_builder.cpp
index ea609d3bdac..efb68f5f632 100644
--- a/be/src/storage/segment/variant/v2/variant_path_builder.cpp
+++ b/be/src/storage/segment/variant/v2/variant_path_builder.cpp
@@ -215,8 +215,16 @@ const DataTypePtr& cached_decimal_type(uint32_t scale) {
return types[scale];
}
-DataTypePtr infer_type(VariantRef value, const DataTypePtr& reusable_type =
nullptr) {
- const ValueKind kind = value_kind(value);
+const DataTypePtr& array_element_type(const DataTypeArray& array) {
+ // DataTypeArray always wraps its element in Nullable. Borrow the element
instead of copying it
+ // through remove_nullable(): element types are process-wide statics
shared by concurrent
+ // flushes, so each shared_ptr copy is a contended reference-count update.
+ return assert_cast<const
DataTypeNullable&>(*array.get_nested_type()).get_nested_type();
+}
+
+// Every scalar storage type is a process-wide static, so it is returned by
reference.
+const DataTypePtr& infer_scalar_type(VariantRef value, ValueKind kind) {
+ DORIS_CHECK(kind != ValueKind::ARRAY);
switch (kind) {
case ValueKind::NULL_VALUE:
return nothing_type();
@@ -272,8 +280,18 @@ DataTypePtr infer_type(VariantRef value, const
DataTypePtr& reusable_type = null
case ValueKind::ARRAY:
break;
}
+ __builtin_unreachable();
+}
+
+DataTypePtr infer_type(VariantRef value, const DataTypePtr& reusable_type =
nullptr) {
+ const ValueKind kind = value_kind(value);
+ if (kind != ValueKind::ARRAY) {
+ return infer_scalar_type(value, kind);
+ }
- DataTypePtr element_type;
+ // Borrow the static element types and only resolve a common type when
elements differ.
+ DataTypePtr promoted_element;
+ const DataTypePtr* element_type = nullptr;
const uint32_t element_count = value.num_elements();
for (uint32_t index = 0; index < element_count; ++index) {
const VariantRef element = value.array_at(index);
@@ -283,17 +301,18 @@ DataTypePtr infer_type(VariantRef value, const
DataTypePtr& reusable_type = null
element.basic_type() == VariantBasicType::OBJECT)) {
return jsonb_type();
}
- DataTypePtr inferred = infer_type(element);
+ const DataTypePtr& inferred = infer_scalar_type(element, element_kind);
if (inferred->get_primitive_type() == INVALID_TYPE) {
continue;
}
- element_type = element_type == nullptr ? std::move(inferred)
- :
path_least_common_type(element_type, inferred);
- }
-
- if (element_type == nullptr) {
- element_type = nothing_type();
+ if (element_type == nullptr) {
+ element_type = &inferred;
+ } else if (element_type->get() != inferred.get()) {
+ promoted_element = path_least_common_type(*element_type, inferred);
+ element_type = &promoted_element;
+ }
}
+ const DataTypePtr& resolved_element = element_type == nullptr ?
nothing_type() : *element_type;
// A path commonly sees the same ARRAY element type on every row. Reuse
the builder's
// DataTypeArray in that case instead of allocating a temporary shared_ptr
per value. The
@@ -302,17 +321,20 @@ DataTypePtr infer_type(VariantRef value, const
DataTypePtr& reusable_type = null
if (const auto* reusable_array =
reusable_type == nullptr ? nullptr
: typeid_cast<const
DataTypeArray*>(reusable_type.get())) {
- const DataTypePtr& reusable_element =
reusable_array->get_nested_type();
- if (reusable_element.get() == element_type.get() ||
- reusable_element->equals(*element_type)) {
+ // Inferred element types are never nullable, so compare against the
unwrapped element;
+ // otherwise every ARRAY value would miss the equality check and pay
+ // get_least_supertype_jsonb() only to rebuild the same type.
+ const DataTypePtr& reusable_element =
array_element_type(*reusable_array);
+ if (reusable_element.get() == resolved_element.get() ||
+ reusable_element->equals(*resolved_element)) {
return reusable_type;
}
- DataTypePtr common_element = path_least_common_type(reusable_element,
element_type);
+ DataTypePtr common_element = path_least_common_type(reusable_element,
resolved_element);
if (reusable_element->equals(*common_element)) {
return reusable_type;
}
}
- return std::make_shared<DataTypeArray>(element_type);
+ return std::make_shared<DataTypeArray>(resolved_element);
}
bool is_small_or_regular_integer(PrimitiveType type) {
@@ -458,8 +480,8 @@ bool value_is_representable(VariantRef value, const
DataTypePtr& target_type) {
if (kind != ValueKind::ARRAY) {
return false;
}
- const DataTypePtr element_type =
- remove_nullable(assert_cast<const
DataTypeArray&>(*target_type).get_nested_type());
+ const DataTypePtr& element_type =
+ array_element_type(assert_cast<const
DataTypeArray&>(*target_type));
const uint32_t count = value.num_elements();
for (uint32_t index = 0; index < count; ++index) {
const VariantRef element = value.array_at(index);
@@ -769,7 +791,7 @@ void append_array(VariantRef value, const DataTypePtr&
target_type, IColumn* tar
const auto& array_type = assert_cast<const DataTypeArray&>(*target_type);
auto& array = assert_cast<ColumnArray&>(*target);
auto& elements = assert_cast<ColumnNullable&>(array.get_data());
- const DataTypePtr element_type =
remove_nullable(array_type.get_nested_type());
+ const DataTypePtr& element_type = array_element_type(array_type);
// infer_type() made the first borrowed pass. Revisit the encoded children
only after path type
// promotion is complete, appending directly without an owning recursive
scratch tree.
const uint32_t count = value.num_elements();
diff --git a/be/test/storage/variant/variant_column_writer_reader_test.cpp
b/be/test/storage/variant/variant_column_writer_reader_test.cpp
index fd9cf208648..15530dd6a58 100644
--- a/be/test/storage/variant/variant_column_writer_reader_test.cpp
+++ b/be/test/storage/variant/variant_column_writer_reader_test.cpp
@@ -28,6 +28,7 @@
#include <thread>
#include "common/config.h"
+#include "core/column/column_array.h"
#include "core/column/column_nullable.h"
#include "core/column/column_string.h"
#include "core/column/column_vector.h"
@@ -809,6 +810,69 @@ TEST(VariantPathBuilderTest,
PreservesIncomingArrayWhenInferredDecimalPromotionO
"[9999999999999999999999999999999999999.9]");
}
+TEST(VariantPathBuilderTest, ArrayPathReusesElementTypeAcrossRows) {
+ VariantBatchBuilder value_builder;
+ const auto append_array = [&](auto&& fill) {
+ auto row = value_builder.begin_row();
+ auto array = row.start_array();
+ fill(row);
+ array.finish();
+ row.finish();
+ };
+ append_array([](auto& row) {
+ row.add_float(1.0F);
+ row.add_float(2.0F);
+ });
+ append_array([](auto& row) {
+ row.add_float(3.0F);
+ row.add_null();
+ });
+ append_array([](auto& row) { row.add_null(); });
+ append_array([](auto& row) { row.add_double(4.5); });
+ append_array([](auto& row) { row.add_float(5.0F); });
+ VariantBatchBuilder values = value_builder.finish_batch();
+
+ const auto element_primitive = [](const DataTypePtr& type) {
+ const DataTypePtr array = remove_nullable(type);
+ return remove_nullable(assert_cast<const
DataTypeArray&>(*array).get_nested_type())
+ ->get_primitive_type();
+ };
+ segment_v2::VariantPathBuilder builder(PathInData("metric"));
+ // FLOAT arrays, arrays with null elements, and all-null arrays share the
first row's type.
+ for (size_t row = 0; row < 3; ++row) {
+ ASSERT_TRUE(builder.append(values.value_at(row), row).ok());
+ EXPECT_EQ(builder.promotion_count(), 0) << "row=" << row;
+ ASSERT_EQ(element_primitive(builder.type()), TYPE_FLOAT) << "row=" <<
row;
+ }
+ ASSERT_TRUE(builder.append(values.value_at(3), 3).ok());
+ EXPECT_EQ(builder.promotion_count(), 1);
+ ASSERT_EQ(element_primitive(builder.type()), TYPE_DOUBLE);
+ // A narrower FLOAT element after promotion reuses the promoted DOUBLE
array type.
+ ASSERT_TRUE(builder.append(values.value_at(4), 4).ok());
+ EXPECT_EQ(builder.promotion_count(), 1);
+ ASSERT_EQ(element_primitive(builder.type()), TYPE_DOUBLE);
+
+ ColumnPtr materialized;
+ ASSERT_TRUE(builder.materialize(&materialized).ok());
+ const auto& array = assert_cast<const ColumnArray&>(
+ assert_cast<const
ColumnNullable&>(*materialized).get_nested_column());
+ const auto& elements = assert_cast<const
ColumnNullable&>(array.get_data());
+ const auto& doubles =
+ assert_cast<const
ColumnFloat64&>(elements.get_nested_column()).get_data();
+ const std::vector<std::optional<double>> expected {1.0, 2.0, 3.0,
std::nullopt,
+ std::nullopt, 4.5, 5.0};
+ EXPECT_EQ(std::vector<uint64_t>(array.get_offsets().begin(),
array.get_offsets().end()),
+ (std::vector<uint64_t> {2, 4, 5, 6, 7}));
+ ASSERT_EQ(elements.size(), expected.size());
+ for (size_t index = 0; index < expected.size(); ++index) {
+ SCOPED_TRACE(testing::Message() << "element=" << index);
+ EXPECT_EQ(elements.is_null_at(index), !expected[index].has_value());
+ if (expected[index].has_value()) {
+ EXPECT_DOUBLE_EQ(doubles[index], *expected[index]);
+ }
+ }
+}
+
TEST(VariantPathBuilderTest,
StringifiesArrayWithoutTreatingExistingNullAsCastFailure) {
VariantBatchBuilder value_builder;
auto row = value_builder.begin_row();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
