[feature](variant) Enable Variant V2 by default - #66858
Draft
eldenmoon wants to merge 2 commits into
Draft
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
1 similar comment
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16987 ms |
Contributor
TPC-DS: Total hot run time: 81251 ms |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
eldenmoon
force-pushed
the
branch-variant-v2-default-on
branch
from
August 18, 2026 10:44
3e2ed7f to
488ebea
Compare
Member
Author
|
run buildall |
eldenmoon
force-pushed
the
branch-variant-v2-default-on
branch
from
August 18, 2026 12:44
488ebea to
2ab8c68
Compare
Contributor
TPC-H: Total hot run time: 17509 ms |
Member
Author
|
run buildall |
Contributor
TPC-DS: Total hot run time: 83404 ms |
Contributor
ClickBench: Total hot run time: 14.58 s |
Contributor
TPC-H: Total hot run time: 17370 ms |
Contributor
TPC-DS: Total hot run time: 83633 ms |
Contributor
ClickBench: Total hot run time: 14.68 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Member
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 17287 ms |
Contributor
TPC-DS: Total hot run time: 83206 ms |
Contributor
ClickBench: Total hot run time: 14.8 s |
Issue Number: None Related PR: apache#66204 Problem Summary: ColumnVariantV2 is implemented in master but remains opt-in through Config.enable_variant_v2, so normal Variant planning and storage continue to select the legacy path. Enable the existing V2 path by default while preserving enable_variant_v2=false as an explicit fallback. Adapt affected regression inputs to use parse_to_variant in V2 mode, keep shared V1/V2 result oracles representation-independent where serialization differs, and retain legacy-only cast coverage without changing production cast behavior. Enable ColumnVariantV2 by default for Variant execution and storage. The legacy path remains available with enable_variant_v2=false. - Test: - Full ASAN BE+FE build without -j - FE ConfigTest (7/7) - FE VariantEqualityContextTest (3/3) - BE focused unit tests (78/78) - Original affected regression suites (24/24, including two export suites against local MinIO) - Additional CI regression suites (8/8 with Variant V2 enabled, plus V1 fallback matrix) - Regression Groovy compilation - clang-format, check-format, and git diff --check - Behavior changed: Yes (ColumnVariantV2 is enabled by default; explicit false remains supported) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66858 Problem Summary: Variant has moved to V2-only execution and storage. The old ColumnVariant V1 implementation kept duplicate factories, cast/load routing, writer/readers, and regression toggles behind enable_variant_v2. This refactor removes the V1 column/data-type/serde implementation, makes Variant factories and storage boundaries produce and accept ColumnVariantV2 only, lets non-V2 compute carriers fail at V2-only boundaries, and updates BE/FE/regression tests to validate V2-only behavior. String-like inputs to Variant are parsed directly into V2 encoded JSON so storage and element access no longer depend on the legacy ColumnVariant carrier. ### Release note Variant now uses V2 only; the legacy enable_variant_v2 FE config and ColumnVariant V1 implementation are removed. ### Check List (For Author) - Test: Regression test / Unit Test - Unit Test: BUILD_TYPE=ASAN GLIBC_COMPATIBILITY=OFF ./run-be-ut.sh --run --filter=ColumnVariantV2Test.*:VariantPathBuilderTest.*:VariantShredderTest.*:VariantColumnWriterReaderTest.*:HierarchicalDataIteratorTest.*:VariantUtilTest.*:VariantDocModeCompactionTest.*:NestedGroupProviderTest.*:DefaultNestedGroupReadProviderTest.*:VariantRowStoreTest.*:DataTypeVariantV2SerDe*:*VariantV2*; BUILD_TYPE=ASAN GLIBC_COMPATIBILITY=OFF ./run-be-ut.sh --run --filter=BinaryColumnExtractIteratorV2Test.*; BUILD_TYPE=ASAN GLIBC_COMPATIBILITY=OFF ./run-be-ut.sh --run --filter=CastVariantV2ToTest.*:VariantElementV2EncodedTest.*:VariantElementV2TypedTest.*; BUILD_TYPE=ASAN GLIBC_COMPATIBILITY=OFF DISABLE_BUILD_UI=ON ./run-fe-ut.sh --run org.apache.doris.nereids.trees.expressions.functions.agg.CountTest,org.apache.doris.nereids.load.VariantLoadParseInjectionTest,org.apache.doris.datasource.scan.PluginDrivenScanNodeCompatibilityTest - Regression test: ./run-regression-test.sh --run -d variant_p1 passed; ./run-regression-test.sh --run -d variant_p0 passed all runnable suites except regression-test/suites/variant_p0/doc_mode/test_outfile_csv_variant_type.groovy, which failed on external OSS InvalidAccessKeyId 403; affected variant_p0 cases were rerun individually after output regeneration. - Build: BUILD_TYPE=ASAN GLIBC_COMPATIBILITY=OFF DISABLE_BUILD_UI=ON ./build.sh --be --fe passed; default GLIBC compatibility link failed on duplicate getrandom from the local toolchain, and FE UI build failed with Node 16.3.0 missing node:util.styleText. - Behavior changed: Yes. Variant no longer supports the legacy V1 ColumnVariant implementation or enable_variant_v2 fallback. - Does this need documentation: No
eldenmoon
force-pushed
the
branch-variant-v2-default-on
branch
from
September 1, 2026 02:05
b4d155b to
e9e58c2
Compare
Member
Author
|
run buildall |
eldenmoon
commented
Sep 1, 2026
| // be deleted without changing the V2 representation. | ||
| // Transitional field-map representation kept only for Field API callers. | ||
| VariantField(VariantMap legacy); | ||
| bool is_legacy() const noexcept; |
eldenmoon
commented
Sep 1, 2026
|
|
||
| // A moved-from V1 field is still a valid empty V1 map, matching std::map move semantics. | ||
| bool _legacy_representation = false; | ||
| std::unique_ptr<VariantMap> _legacy; |
Member
Author
There was a problem hiding this comment.
这里是不是没必要用VariantMap这个？
eldenmoon
commented
Sep 1, 2026
| // 2. Finalize the Variant column to speed up | ||
| for (auto& data : block) { | ||
| data.column = IColumn::mutate(std::move(data.column))->convert_column_if_overflow(); | ||
| if (p._need_finalize_variant_column) { |
Member
Author
There was a problem hiding this comment.
这里的 finalize 逻辑应该也不需要了， 评估一下，如果没用了就删除
eldenmoon
commented
Sep 1, 2026
|
|
||
| DataTypePtr get_return_type_impl(const DataTypes& arguments) const override { | ||
| DataTypePtr arg_0 = remove_nullable(arguments[0]); | ||
| DCHECK(arg_0->get_primitive_type() == TYPE_ARRAY || arg_0->get_primitive_type() == TYPE_MAP) |
eldenmoon
commented
Sep 1, 2026
| ? CastWrapper::variant_v2_internal::ForcedNulls {} | ||
| : CastWrapper::variant_v2_internal::ForcedNulls { | ||
| data.nested_nullmap_data, data.nested_col->size()}; | ||
| Status status; |
Member
Author
There was a problem hiding this comment.
为什么是在这里 cast，而不是在 CAST 函数里？如果要修改，形成单独的 commit
eldenmoon
commented
Sep 1, 2026
| } else if (primitive == TYPE_ARRAY) { | ||
| RETURN_IF_ERROR(cast_array_to_variant(source_ptr, from_type, rows, | ||
| forced_nulls(null_map, rows), &output)); | ||
| } else if (is_string_type(primitive)) { |
Member
Author
There was a problem hiding this comment.
这里是错误的，String 不应该解析，这个不是 ColumnVariantV2 的正确行为，评估一下为什么引入，删除是什么行为，应该删除，如果可以删除，形成单独的 commit 来 review
eldenmoon
commented
Sep 1, 2026
| BinaryColumnExtractIterator::BinaryColumnExtractIterator(std::string_view path, | ||
| BinaryColumnCacheSPtr sparse_column_cache, | ||
| const StorageReadOptions* opts, | ||
| bool use_variant_v2) |
eldenmoon
commented
Sep 1, 2026
| @@ -57,9 +50,9 @@ Status HierarchicalDataIterator::create(ColumnIteratorUPtr* reader, int32_t col_ | |||
| ColumnReaderCache* column_reader_cache, | |||
| OlapReaderStatistics* stats, ReadType read_type, | |||
| bool use_variant_v2, const io::IOContext* io_ctx) { | |||
eldenmoon
commented
Sep 1, 2026
| // TODO: Remove it together with legacy ColumnVariant readers. | ||
| DORIS_CHECK(!target_col.variant_is_v2()); | ||
| plan->kind = ReadKind::ROOT_FLAT; | ||
| plan->kind = ReadKind::HIERARCHICAL; |
Member
Author
There was a problem hiding this comment.
// ROOT_FLAT reads the persisted root column itself. It does not rebuild root
vfrom
// regular extracted columns such asv.keep/v.owner; only the optional root-merge
// wrapper below may fold NestedGroup data back into the root view.
Member
Author
There was a problem hiding this comment.
这里是否会 rebuild root v？先说明如何解决
eldenmoon
commented
Sep 1, 2026
| // deleted and Variant readers always produce ColumnVariantV2. It only selects the in-memory | ||
| // compute destination and must never be serialized into tablet or segment metadata. | ||
| bool _variant_is_v2 = false; | ||
| bool _variant_is_v2 = true; |
eldenmoon
commented
Sep 1, 2026
| } | ||
| } | ||
|
|
||
| Status parse_and_materialize_variant_columns(Block& block, const TabletSchema& tablet_schema, |
Member
Author
There was a problem hiding this comment.
parse_and_materialize_variant_columns 在 ColumnVariantV2 是不是不需要了， VariantParseStage 也不需要了，因为ColumnVariantV2已经解析过了
eldenmoon
commented
Sep 1, 2026
| {"other_1":"33"} | ||
| {"other_1":"34"} | ||
| {"other_1":"35"} | ||
| {"other_1":1} |
Member
Author
There was a problem hiding this comment.
这里输出是否符合预期？引号没了，原始是什么类型
eldenmoon
commented
Sep 1, 2026
| @@ -9,32 +9,28 @@ | |||
| 7 {"a":7,"g":70,"h":700} | |||
Member
Author
There was a problem hiding this comment.
regression-test/data/variant_p0/test_variant_compaction_empty_path_bug.out 输出为什么empty_key_value 没了？
eldenmoon
commented
Sep 1, 2026
| 13 130 | ||
| 16 16 | ||
|
|
||
| -- !empty_key -- |
eldenmoon
commented
Sep 1, 2026
| @@ -1,10 +0,0 @@ | |||
| -- This file is automatically generated. You should know what you did if you want to edit this | |||
eldenmoon
commented
Sep 1, 2026
| @@ -43,7 +40,6 @@ suite("test_all_prdefine_type_to_sparse", "p0,nonConcurrent") { | |||
| 'decimal32_*':decimalv3(8,2), | |||
| 'decimal64_*':decimalv3(16,9), | |||
| 'decimal128_*':decimalv3(36,9), | |||
| 'decimal256_*':decimalv3(70,60), | |||
Member
Author
There was a problem hiding this comment.
decimal256_ 应该报错，增加报错的 case
eldenmoon
commented
Sep 1, 2026
| (626, ${variantV2Function}('{"other_1": "11"}')),(627, ${variantV2Function}('{"other_1": "12"}')),(628, ${variantV2Function}('{"other_1": "13"}')),(629, ${variantV2Function}('{"other_1": "14"}')),(630, ${variantV2Function}('{"other_1": "15"}')),(631, ${variantV2Function}('{"other_1": "16"}')),(632, ${variantV2Function}('{"other_1": "17"}')),(633, ${variantV2Function}('{"other_1": "18"}')),(634, ${variantV2Function}('{"other_1": "19"}')),(635, ${variantV2Function}('{"other_1": "20"}')), | ||
| (636, ${variantV2Function}('{"other_1": "21"}')),(637, ${variantV2Function}('{"other_1": "22"}')),(638, ${variantV2Function}('{"other_1": "23"}')),(639, ${variantV2Function}('{"other_1": "24"}')),(640, ${variantV2Function}('{"other_1": "25"}')),(641, ${variantV2Function}('{"other_1": "26"}')),(642, ${variantV2Function}('{"other_1": "27"}')),(643, ${variantV2Function}('{"other_1": "28"}')),(644, ${variantV2Function}('{"other_1": "29"}')),(645, ${variantV2Function}('{"other_1": "30"}')), | ||
| (646, ${variantV2Function}('{"other_1": "31"}')),(647, ${variantV2Function}('{"other_1": "32"}')),(648, ${variantV2Function}('{"other_1": "33"}')),(649, ${variantV2Function}('{"other_1": "34"}')),(650, ${variantV2Function}('{"other_1": "35"}')); """ | ||
| sql """ insert into ${tableName} values (616, ${variantV2Function}('{"other_1": 1}')),(617, ${variantV2Function}('{"other_1": 2}')),(618, ${variantV2Function}('{"other_1": 3}')),(619, ${variantV2Function}('{"other_1": 4}')),(620, ${variantV2Function}('{"other_1": 5}')),(621, ${variantV2Function}('{"other_1": 6}')),(622, ${variantV2Function}('{"other_1": 7}')),(623, ${variantV2Function}('{"other_1": 8}')),(624, ${variantV2Function}('{"other_1": 9}')),(625, ${variantV2Function}('{"other_1": 10}')), |
Contributor
FE UT Coverage ReportIncrement line coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #66204
Problem Summary: ColumnVariantV2 is implemented in master but remains opt-in through Config.enable_variant_v2, so normal Variant planning and storage continue to select the legacy path. Make the existing V2 path the default while preserving enable_variant_v2=false as an explicit fallback.
Enabling V2 by default exposed regression cases that assumed legacy implicit string-to-Variant assignment or legacy output representation. Update those cases following variant_p0 conventions: construct V2 values with parse_to_variant and normalize only representation-dependent expectations. Production Cast behavior is unchanged. Keep the hierarchical iterator test portable across standard library implementations.
Release note
Enable ColumnVariantV2 by default for Variant execution and storage. The legacy path remains available with enable_variant_v2=false.
Check List (For Author)