eldenmoon commented on code in PR #66204:
URL: https://github.com/apache/doris/pull/66204#discussion_r3672201902
##########
be/test/storage/variant/index_storage_variant_sparse_stats_test.cpp:
##########
@@ -249,27 +297,128 @@ void
IndexStorageVariantSparseStatsTest::run_sparse_stats_limit_boundary_case(
EXPECT_TRUE(std::all_of(missing_values.begin(), missing_values.end(),
[](const auto& value) {
return !value.has_value();
})) << describe_optional_string_values(missing_values);
+
+ auto v2_readable =
+ inject_reader_schema_for_rowsets(missing_readable.value(),
std::move(v2_schema));
+ ASSERT_TRUE(v2_readable.has_value()) << v2_readable.error();
+ const int32_t v2_child_column_id = column_id_by_path("v.b.c");
+ const int32_t v2_missing_column_id = column_id_by_path("v.b.missing");
+ ASSERT_GE(v2_child_column_id, 0) << dump_schema_paths(*tablet_schema());
+ ASSERT_GE(v2_missing_column_id, 0) << dump_schema_paths(*tablet_schema());
+
+ auto read_v2_path = [&](int32_t column_id) {
+ IndexReadOptions read_options;
+ read_options.need_ordered_result = true;
+ read_options.return_columns = {0, static_cast<uint32_t>(column_id)};
+ read_options.collect_variant_values = true;
+ read_options.use_variant_v2 = true;
+ return read_rowsets(v2_readable.value(), std::move(read_options));
+ };
+
+ auto v2_child = read_v2_path(v2_child_column_id);
+ ASSERT_TRUE(v2_child.has_value()) << v2_child.error();
+ ASSERT_EQ(v2_child->rows_read, 5);
+ ASSERT_TRUE(v2_child->variant_v2_output_uids.contains(-1));
+ ASSERT_TRUE(v2_child->variant_values_by_uid.contains(-1));
+ const auto& v2_child_values = v2_child->variant_values_by_uid.at(-1);
+ ASSERT_EQ(v2_child_values.size(), 5);
+ EXPECT_EQ(std::count(v2_child_values.begin(), v2_child_values.end(),
+ std::optional<std::string> {R"("child-0")"}),
+ 1)
+ << describe_optional_string_values(v2_child_values);
+ const bool stats_below_limit =
Review Comment:
[P1] 同一逻辑缺失路径不能因为 sparse stats/read plan 不同而改变 SQL NULL 语义。
这里明确按 `stats_below_limit` 分支：能选 BINARY_EXTRACT 时期望 `std::nullopt`，stats
截断、退到 HIERARCHICAL 时却期望字符串 `"null"`；下面完全不存在的 path
也同样分叉。`sparse_stats_limit`、external meta 和 compaction 都是内部物理因素，但这个差异可被 `IS
NULL`、`variant_type`、CAST 和序列化直接观察。
测试应对两种 plan 断言相同逻辑结果。实现上需要让 hierarchical assembler 区分“requested path 缺失”与“真实
Variant JSON null”，而不是在 `assemble_hierarchical_row` 没有 emitted value 时一律
`row.add_null()` 后仍写 `outer_null=0`。
##########
regression-test/suites/variant_p0/v2/README.md:
##########
@@ -0,0 +1,166 @@
+# Variant V2 regression coverage
+
+This directory is the V2-read counterpart of a selected `variant_p0` baseline.
Review Comment:
[P1] 请把 `e21f370` 的 compute closure 和整棵 regression 迁移拆成独立 PR。
本 PR 标题/描述及原提交都只处理 legacy segment -> ColumnVariantV2 read adapter（24
files，+5,679/-78）；这个提交却加入 aggregate/cast/`variant_type`/MV/Decimal256 行为和 208 个
regression 文件（228 files，+29,384/-32）。把两类行为放在一起会让 reader 变更无法独立
review、回滚或定位回归，PR checklist 也已经与实际范围不一致。后续 `d694272` 的 reader coverage 可以留在本 PR。
##########
regression-test/suites/variant_p0/v2/README.md:
##########
@@ -0,0 +1,166 @@
+# Variant V2 regression coverage
+
+This directory is the V2-read counterpart of a selected `variant_p0` baseline.
+It contains exactly 105 framework-discovered cases:
+
+- 84 suites that passed the historical `variant_p0` run with V2 reads enabled.
+- 21 suites selected to close the compute, cast, aggregate, MV, and related V2
+ failures from that run.
+
+Every suite enables `enable_variant_v2` at session scope. No suite changes the
+global variable. Storage writes continue to use the legacy `ColumnVariant`
+path. The two self-contained GitHub-data fixtures and
+`variant_compute_v2.groovy` explicitly switch V2 off while preparing legacy
+segments and enable it again before scanning them.
+
+The two former SQL suites `sql/gh_data.sql` and
+`sql/rewrite_or_to_in.sql` are Groovy suites here. Each owns a different table
+and loads `ghdata_sample.json` itself, so either suite can run alone and the
two
+can run without sharing mutable state.
+
+## What is covered
+
+- Segment reads of the whole Variant root, dense typed subcolumns, sparse
+ subcolumns, missing paths, and SQL NULL rows.
+- Subpath extraction, nested selectors, array selectors and casts, predicates,
+ CTE/UNION propagation, and subpath pruning.
+- `parse_to_variant`, strict and permissive casts, scalar/array/JSON/JSONB
+ conversion, and explicit string conversion of Variant subpaths.
+- `variant_type`, equality/hash contexts, `GROUP BY`, `DISTINCT`,
+ `COUNT(DISTINCT)`, and aggregate-key `REPLACE` reads in normal and doc mode.
+- Sync MV and MTMV creation/query compatibility from a V2-enabled session,
+ including the row-store crash reproducer. MV/MTMV materialization remains on
+ the V1 write path; this does not claim V2 materialized storage.
+- A partial doc-mode matrix: doc materialization threshold, delete/partial
+ update, MOW, aggregate reads, predefined patterns and type indexes, and
+ predefined schema change.
+- A partial sparse matrix: typed-to-sparse transitions, timestamp sparse
+ values, empty-key sparse buckets, sparse external metadata, predefined type
+ indexes, and typed/sparse conflict cases.
+- Bloom-filter and inverted-index compatibility checks, schema changes, MOW,
+ partition reads, row-id TopN reads, RQG queries, and TPCH queries over
Variant.
+
+Ninety-three cases contain functional assertions. Twelve setup-only cases
Review Comment:
[P2] 不要为了保持历史 inventory 长期维护第二棵近似测试树和无断言 case。
105 个 case 中有 103 个能映射回现有 `variant_p0` 路径；至少
`variant_parse_functions.groovy`、`test_variant_count_distinct.groovy`、`test_variant_ordering_comparison_error.groovy`
是 100% 相同且原文件本来就开启 V2，可以直接复用。两份 `variant_compute_v2` 已经发生实际漂移：根目录仍期望
`variant_type` 抛错，新副本期望成功。
至少先删除完全重复的 V2-only suite 和这里列出的 12 个无功能断言 case。其余迁移放到独立 PR，再让 V1/V2 使用共享
case body 或逐步把 V2 变成 canonical suite，只为真正的 legacy-segment 兼容性保留少量 V1
fixture；这也更符合未来删除 V1 的方向。
##########
be/src/exprs/function/cast/variant_v2/cast_variant_to_array.cpp:
##########
@@ -73,6 +77,58 @@ void append_collected_value(CollectedArrayNode* node,
VariantRef value, bool for
node->offsets.push_back(node->child->size());
}
+bool is_string_value(VariantRef value) {
+ return value.basic_type() == VariantBasicType::SHORT_STRING ||
+ (value.basic_type() == VariantBasicType::PRIMITIVE &&
+ value.primitive_id() == VariantPrimitiveId::STRING);
+}
+
+Status cast_strings_to_array(FunctionContext* context, const ColumnPtr& source,
Review Comment:
[P2] 这里可以复用 scalar cast 已有的 non-strict executor。
这个函数与 `cast_variant_to_scalar.cpp:282-305` 重复了 clone context、关闭 strict
mode、构造 temporary Block、`prepare_unpack_dictionaries`、把 INVALID_ARGUMENT 转成
all-null、以及取 nullable output 的整套 policy。建议把通用部分提到
`cast_variant_v2_internal`（参数化 source name/最终包装差异），array 和 scalar 共用，避免后续
strict/error/null policy 在两处漂移。
##########
be/src/exprs/function/function_variant_type.cpp:
##########
@@ -29,6 +33,195 @@ class FunctionContext;
namespace doris {
+namespace {
+
+std::string_view encoded_variant_type_word(VariantRef value) {
+ switch (value.basic_type()) {
+ case VariantBasicType::SHORT_STRING:
+ return "string";
+ case VariantBasicType::OBJECT:
+ return "object";
+ case VariantBasicType::ARRAY:
+ return "array";
+ case VariantBasicType::PRIMITIVE:
+ break;
+ }
+
+ switch (value.primitive_id()) {
+ case VariantPrimitiveId::NULL_VALUE:
+ return "null";
+ case VariantPrimitiveId::TRUE_VALUE:
+ case VariantPrimitiveId::FALSE_VALUE:
+ return "bool";
+ case VariantPrimitiveId::INT8:
+ return "tinyint";
+ case VariantPrimitiveId::INT16:
+ return "smallint";
+ case VariantPrimitiveId::INT32:
+ return "int";
+ case VariantPrimitiveId::INT64:
+ return "bigint";
+ case VariantPrimitiveId::DOUBLE:
+ return "double";
+ case VariantPrimitiveId::DECIMAL4:
+ case VariantPrimitiveId::DECIMAL8:
+ case VariantPrimitiveId::DECIMAL16:
+ static_cast<void>(value.get_decimal());
+ return "decimal";
+ case VariantPrimitiveId::DATE:
+ return "date";
+ case VariantPrimitiveId::TIMESTAMP_MICROS:
+ case VariantPrimitiveId::TIMESTAMP_NANOS:
+ return "timestamp";
+ case VariantPrimitiveId::TIMESTAMP_NTZ_MICROS:
+ case VariantPrimitiveId::TIMESTAMP_NTZ_NANOS:
+ return "timestamp_ntz";
+ case VariantPrimitiveId::FLOAT:
+ return "float";
+ case VariantPrimitiveId::BINARY:
+ return "binary";
+ case VariantPrimitiveId::STRING:
+ return "string";
+ case VariantPrimitiveId::TIME_NTZ_MICROS:
+ return "time";
+ case VariantPrimitiveId::UUID:
+ return "uuid";
+ }
+ DORIS_CHECK(false) << "validated Variant primitive id has no type word";
+ return {};
+}
+
+std::string_view typed_integer_type_word(const VariantScalarRef& scalar) {
+ switch (scalar.encoded_size()) {
+ case 2:
+ return "tinyint";
+ case 3:
+ return "smallint";
+ case 5:
+ return "int";
+ case 9:
+ return "bigint";
+ default:
+ DORIS_CHECK(false) << "typed Variant integer has invalid encoded size "
+ << scalar.encoded_size();
+ return {};
+ }
+}
+
+template <PrimitiveType Type>
+std::string_view typed_value_type_word(const VariantScalarRef& scalar) {
+ if constexpr (Type == TYPE_TINYINT || Type == TYPE_SMALLINT || Type ==
TYPE_INT ||
+ Type == TYPE_BIGINT) {
+ return typed_integer_type_word(scalar);
+ }
+ DORIS_CHECK(false) << "typed Variant type word does not depend on its
value";
+ return {};
+}
+
+template <PrimitiveType Type>
+std::string_view typed_fixed_type_word() {
+ if constexpr (Type == TYPE_BOOLEAN) {
+ return "bool";
+ } else if constexpr (Type == TYPE_FLOAT) {
+ return "float";
+ } else if constexpr (Type == TYPE_DOUBLE) {
+ return "double";
+ } else if constexpr (Type == TYPE_DECIMALV2 || Type == TYPE_DECIMAL32 ||
+ Type == TYPE_DECIMAL64 || Type == TYPE_DECIMAL128I) {
+ return "decimal";
+ } else if constexpr (Type == TYPE_DATE || Type == TYPE_DATEV2) {
+ return "date";
+ } else if constexpr (Type == TYPE_DATETIME || Type == TYPE_DATETIMEV2) {
+ return "timestamp_ntz";
+ } else if constexpr (Type == TYPE_TIMESTAMPTZ) {
+ return "timestamp";
+ } else if constexpr (Type == TYPE_CHAR || Type == TYPE_VARCHAR || Type ==
TYPE_STRING ||
+ Type == TYPE_IPV4 || Type == TYPE_IPV6 || Type ==
TYPE_DECIMAL256) {
+ return "string";
+ }
+ DORIS_CHECK(false) << "typed Variant type word depends on its value";
+ return {};
+}
+
+ColumnPtr execute_variant_type_v2(const ColumnVariantV2& source) {
Review Comment:
[P1] `variant_type` 的公开结果不应只因物理执行列从 V1 切到 V2 就换契约。
同一个 legacy segment/SQL，V1 分支返回每个 path 的 JSON type map（本文件 239-306），这里却只返回
root word（`object`/`array`/scalar）；typed integer 还会按实际编码宽度报告
tinyint/smallint。打开 session switch 即可观察差异，未来删除 V1 后旧契约会直接消失，但当前 PR 的 Release
note 仍是 None。
如果兼容性是目标，请用 V2 view 复现原有可观察 contract；如果这是有意的 API 重定义，应放在独立 compute PR，明确
breaking/experimental contract、Release note/docs，并加入同一 SQL 在 switch false/true
下的兼容性测试，不能只更新一套 V2 golden。
##########
be/src/exprs/function/function_variant_type.cpp:
##########
@@ -29,6 +33,195 @@ class FunctionContext;
namespace doris {
+namespace {
+
+std::string_view encoded_variant_type_word(VariantRef value) {
+ switch (value.basic_type()) {
+ case VariantBasicType::SHORT_STRING:
+ return "string";
+ case VariantBasicType::OBJECT:
+ return "object";
+ case VariantBasicType::ARRAY:
+ return "array";
+ case VariantBasicType::PRIMITIVE:
+ break;
+ }
+
+ switch (value.primitive_id()) {
+ case VariantPrimitiveId::NULL_VALUE:
+ return "null";
+ case VariantPrimitiveId::TRUE_VALUE:
+ case VariantPrimitiveId::FALSE_VALUE:
+ return "bool";
+ case VariantPrimitiveId::INT8:
+ return "tinyint";
+ case VariantPrimitiveId::INT16:
+ return "smallint";
+ case VariantPrimitiveId::INT32:
+ return "int";
+ case VariantPrimitiveId::INT64:
+ return "bigint";
+ case VariantPrimitiveId::DOUBLE:
+ return "double";
+ case VariantPrimitiveId::DECIMAL4:
+ case VariantPrimitiveId::DECIMAL8:
+ case VariantPrimitiveId::DECIMAL16:
+ static_cast<void>(value.get_decimal());
+ return "decimal";
+ case VariantPrimitiveId::DATE:
+ return "date";
+ case VariantPrimitiveId::TIMESTAMP_MICROS:
+ case VariantPrimitiveId::TIMESTAMP_NANOS:
+ return "timestamp";
+ case VariantPrimitiveId::TIMESTAMP_NTZ_MICROS:
+ case VariantPrimitiveId::TIMESTAMP_NTZ_NANOS:
+ return "timestamp_ntz";
+ case VariantPrimitiveId::FLOAT:
+ return "float";
+ case VariantPrimitiveId::BINARY:
+ return "binary";
+ case VariantPrimitiveId::STRING:
+ return "string";
+ case VariantPrimitiveId::TIME_NTZ_MICROS:
+ return "time";
+ case VariantPrimitiveId::UUID:
+ return "uuid";
+ }
+ DORIS_CHECK(false) << "validated Variant primitive id has no type word";
+ return {};
+}
+
+std::string_view typed_integer_type_word(const VariantScalarRef& scalar) {
Review Comment:
[P2] 不要用 wire `encoded_size()` 的 2/3/5/9 魔数反推 primitive identity。
`VariantScalarRef` 内部已经持有准确的 physical primitive id，而本文件上方又维护了一张
`VariantPrimitiveId -> type word` 表；`typed_fixed_type_word()` 还是第三张 typed
whitelist。建议在 core Variant helper 暴露稳定的 physical id/classification，并集中提供一套
type-word 映射，让 encoded/typed 两态共用。否则修改整数编码或增加 typed identity 时，需要同步多张
switch，漏改会变成运行期 `DORIS_CHECK`。
--
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]
