Repository navigation
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Adds a default-disabled backend ColumnVariantV2 execution path for compute-only use without changing storage behavior. The implementation provides canonical Variant encoding and serde, nullable parse and cast functions, equality and hash semantics, aggregation, joins, conditional expressions, set operations, and explode. It also aligns mixed encoded and typed Variant equality in Nereids and declares Variant-to-scalar casts nullable when shape or range conversion can fail. Window functions remain unsupported in this phase. ### Release note Adds an experimental, default-disabled ColumnVariantV2 compute execution path. ### Check List (For Author) - Test: BUILD_TYPE=ASAN ./build.sh --be; ./build.sh --fe; BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run --filter=VariantNativeV2PathTest.*:VariantElementV2TypedTest.* (5 passed); focused FE UT for Variant equality, runtime filters, count, element_at, type coercion, and function registry (80 passed); CastTest (17 passed); variant_compute_v2, variant_parse_functions, test_variant_equality_contexts, and test_variant_ordering_comparison_error regressions; build-support/clang-format.sh; build-support/check-format.sh. clang-tidy was attempted but could not analyze the files because the local toolchain could not resolve stddef.h. - Behavior changed: Yes (adds an opt-in ColumnVariantV2 compute path; legacy behavior remains the default) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: ColumnVariantV2 regression suites changed a process-wide BE config while P0 suites ran concurrently. Legacy Variant storage suites could then mix ColumnVariant and ColumnVariantV2 assumptions and crash in _parse_and_materialize_variant_columns. Restrict this phase to canonical hash and arena serialization for grouping, distinct aggregation, and set operators; reject root Variant comparison predicates and join keys; remove the Variant join/runtime-filter path; and isolate config-changing V2 suites as nonConcurrent. ### Release note ColumnVariantV2 remains experimental and disabled by default. This phase supports compute-only canonical hashing and serialization but not root Variant comparison predicates or join keys. ### Check List (For Author) - Test: BUILD_TYPE=ASAN ./build.sh --be; ./build.sh --fe; targeted FE UT (23 tests); targeted ASAN BE UT (16 tests); test_variant_equality_contexts regression - Behavior changed: Yes (root Variant comparison predicates and join keys are rejected; canonical hash-based compute operators remain supported) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Variant V2 still exposed a legacy per-row builder and unused native kernels, accepted Decimal256 through fallback paths, and lacked an exact canonical distinct state plus complete E/T and Parquet boundary coverage. This change rejects unsupported Decimal256 inputs, consolidates typed construction, removes dead adapters and kernels, moves builder callers to VariantBlockBuilder, and adds exact canonical distinct and cross-implementation golden coverage. Direct Variant comparisons and joins remain rejected by FE. ### Release note Variant V2 supports exact distinct aggregation, rejects Decimal256 inputs, and removes unused legacy builder and native APIs. ### Check List (For Author) - Test: ASAN BE+FE build; CountTest FE UT (8/8); focused Variant BE UT (80/80 plus final uniq 4/4); variant_compute_v2, test_variant_equality_contexts, and test_variant_count_distinct regression suites; Parquet Java extended golden; clang-format/check-format - Behavior changed: Yes (Decimal256-to-Variant rejects; exact Variant distinct is enabled; direct Variant comparisons and joins remain rejected.) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: ColumnVariantV2 exposed stale variant_p0 expectations that relied on implicit Variant-to-JSON conversion, root Variant predicates, and rejection of compatible Variant layout inserts. Align the suites with native subpath access or explicit JSONB bridges, keep unsupported root comparison coverage centralized, and add positive golden coverage for compatible layout and transaction batch inserts. ### Release note None ### Check List (For Author) - Test: Focused changed suites plus test_variant_equality_contexts and test_variant_count_distinct passed; full variant_p0 ran 146 suites with 140 passed and 6 environment-only failures (one InvalidAccessKeyId and five DebugPoint remove HTTP 500 failures), fatal 0 - Behavior changed: No (regression expectations now match ColumnVariantV2 behavior) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Address follow-up review feedback for Variant V2 source placement, cast/data-type separation, encoded-block ownership, value-ref naming, and typed encoding overhead. VariantEncodedBlock now directly owns its buffers, column adapters consume the owner, and tests cover empty, small, and moved blocks. ### Release note None ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - Behavior changed: - [x] No. - [ ] Yes. - Does this need documentation? - [x] No. - [ ] Yes.
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Variant typed encoding and canonical operations maintained separate scalar descriptions. Introduce a single physical-aware VariantScalarRef shared by batch encoding and canonical normalization, remove VariantScalarEncodingPlan, and make typed adapters produce one scalar per row. ### Release note None ### Check List (For Author) - Test: Unit Test - BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run --filter='VariantBatchBuilderTest.*:VariantBatchBuilderAddValueTest.*:VariantCanonicalTest.*:ColumnVariantV2Test.*:CastVariantV2ToTest.*:CastVariantV2FromTest.*' - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Rename ParseToVariantErrorToNull to the conventional TryParseToVariant name across BE registration, Nereids, tests, and regression suites. The SQL function is now exposed as try_parse_to_variant. ### Release note None ### Check List (For Author) - Test: Unit Test - BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run --filter='FunctionVariantParseTest.*' - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.analysis.FunctionRegistryTest#testVariantParseFunctions - Behavior changed: Yes - Rename parse_to_variant_error_to_null to try_parse_to_variant. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Route multi_distinct_count to AggregateFunctionUniqVariant only for DataTypeVariantV2. Reject legacy Variant with INVALID_ARGUMENT instead of using the V2 canonical serialization path. ### Release note None ### Check List (For Author) - Test: Unit Test - BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run --filter='AggregateFunctionUniqVariantTest.*' - Behavior changed: Yes - multi_distinct_count rejects legacy Variant and supports Variant V2. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65561 Problem Summary: Add variant_get with constant JSONPath extraction for query-time Variant values while preserving SQL NULL versus Variant null semantics and consistent encoded/typed behavior. Add a recursive-schema prototype currently exposed as variant_typeof, plus FE registration and focused BE, FE, and regression coverage. The function name and compatibility contract remain a Draft design decision because StarRocks variant_typeof returns only the top-level runtime type. Add Release BE microbenchmarks for parse_to_variant, path hits and misses, arrays, object width, recursive schema inference, and variant_get plus CAST. The log-shaped case covers 32 fields and BIGINT, DOUBLE, STRING, and BOOLEAN extraction. The functions in this stacked change accept compute-only Variant values; persisted Variant columns are outside the current scope. ### Release note Draft API: add query-time Variant extraction and type inspection functions. Public names and final semantics are not yet fixed. ### Check List (For Author) - Test - [x] Unit Test - [x] Manual test - [ ] Regression test - [ ] No need to test or manual test. - Behavior changed: - [ ] No. - [x] Yes. Adds query-time Variant functions and BE benchmarks. - Does this need documentation? - [ ] No. - [x] Yes. Follow-up documentation is required.
Owner
Author
|
run buildall |
eldenmoon
marked this pull request as ready for review
July 27, 2026 07:01
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f66b79ca98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+90
to
+95
| if (common_escape) { | ||
| output->push_back('\\'); | ||
| output->push_back(escaped); | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
When a quoted member uses a JSON Unicode escape, such as $["\u0061"] for key a, append_normalized_escape excludes u and returns false, causing a valid constant path to fail with Invalid JSONPath before it reaches JsonbPath; the existing JsonbPath::remove_escapes() already supports \u00XX. Preserve and validate Unicode escape sequences during normalization so escaped keys remain addressable.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
eldenmoon
marked this pull request as draft
July 27, 2026 07:21
eldenmoon
force-pushed
the
feature-column-variant-v2
branch
2 times, most recently
from
July 27, 2026 13:52
784cff1 to
a932fb5
Compare
eldenmoon
pushed a commit
that referenced
this pull request
Oct 19, 2026
…pache#69100) ### What problem does this PR solve? Problem Summary: `RleEncoder::FlushRepeatedRun()` wrote `repeat_count_ << 1` into an `int32_t`. `repeat_count_` was an `int`, so a repeated run of 2^30 or more values overflowed the indicator into a negative number. `BitWriter::PutVlqInt()` then took that as an `int32_t` and shifted it right arithmetically: ```cpp inline void BitWriter::PutVlqInt(int32_t v) { while ((v & 0xFFFFFF80) != 0L) { PutAligned<uint8_t>((v & 0x7F) | 0x80, 1); v >>= 7; } PutAligned<uint8_t>(v & 0x7F, 1); } ``` An arithmetic right shift of a negative value converges to `-1` and stays there, and `-1 & 0xFFFFFF80` is non-zero, so **the loop never exits**. It appends `0xFF` to the output buffer forever, pinning a core at 100% and growing the buffer without bound. #### How it was hit A base compaction in a production cluster wedged on a table with many nullable `text` columns. Two `BaseCompactionT` threads sat in this stack for over a month: ``` #0 inline_memcpy (size=348057208) #2 doris::faststring::GrowArray (newcapacity=502825728) #4 doris::BitWriter::GetNextBytePtr (num_bytes=1) #5 doris::BitWriter::PutAligned<unsigned char> (val=255 '\377') #6 doris::BitWriter::PutVlqInt (v=-1) bit_stream_utils.inline.h:91 #7 doris::RleEncoder<bool>::FlushRepeatedRun rle_encoding.h:499 #9 doris::segment_v2::NullBitmapBuilder::finish #11 doris::segment_v2::ScalarColumnWriter::finish #15 doris::Merger::vertical_compact_one_group (is_key=false, max_rows_per_segment=1073741824) apache#20 doris::CloudBaseCompaction::execute_compact ``` `v=-1` together with `val=255` pins it down exactly: the loop is emitting `0xFF` with `v` stuck at `-1`. Two existing behaviours combined to produce a 2^30-value run: - Vertical compaction derives the segment row limit from `vertical_compaction_max_segment_size / avg_bytes_per_row` ([`Compaction::get_avg_segment_rows()`](https://github.com/apache/doris/blob/master/be/src/olap/compaction.cpp)). For this table the average row size integer-divided to `0`, so the divisor degenerated to `1` and the limit became the config default `1073741824` = 2^30 rows — visible as `max_rows_per_segment=1073741824` in the stack above. - `ScalarColumnWriter::append_nulls()` never checks whether the page is full, unlike `append_data()`, and NULLs never reach the page builder. So an all-NULL column accumulated a single null-bitmap run spanning the whole 2^30-row segment in one page. Because the stuck tasks kept the base compaction slots occupied (`max_base_compaction_task_num_per_disk` defaults to 2), `_generate_cloud_compaction_tasks()` computed `base_n = 0` and stopped picking any tablet, so **no base compaction ran on that backend again** and compaction scores climbed to the cap across the instance. ### What this PR changes 1. **`PutVlqInt()` takes `uint32_t`.** A logical shift always converges to 0, so the loop terminates for every possible input. This is the backstop that makes a hang impossible regardless of what callers pass. 2. **`repeat_count_` widened to `int64_t`, and long runs are split.** Runs longer than `MAX_VALUES_PER_REPEATED_RUN` (2^30 - 1) are emitted as several back-to-back repeated runs of the same value. Readers already merge adjacent runs carrying the same value (`RleDecoder::GetNextRun`), so splitting is invisible to them. The cap keeps the indicator inside `int32_t`'s positive range — exactly the range previous versions could encode — so **output stays byte-identical for every run length that used to work**, and no reader change is needed. 3. **`faststring::reserve()` grows geometrically** via the existing `GrowToAtLeast()` instead of reallocating to exactly the requested capacity. `BitWriter` reserves a handful of bytes at a time, so exact growth reallocated and memcpy'd the entire buffer every 8 bytes, making an n-byte append cost O(n^2). This is what turned the infinite loop into a thread stuck in `memcpy` with a ~500MB buffer, and it slows down every other large RLE buffer as well. Measured: reaching 1 MiB now takes 26 reallocations instead of ~131,000. Deliberately **not** in this PR, each worth its own change: - `ScalarColumnWriter::append_nulls()` still lets a page grow without bound. - `Compaction::get_avg_segment_rows()` still degenerates to the configured byte size when the average row size rounds down to zero. ### Release note Fix a BE hang in RLE encoding when a column produces a run of 2^30 or more identical values (for example an all-NULL column in a very large segment), which could permanently stall base compaction on that backend. ### Check List (For Author) - Test - [x] Unit Test New tests: | Test | Covers | |---|---| | `TestRle.TestRepeatedRunExceedingIndicatorRange` | Run lengths around and far beyond 2^30 encode compactly and decode back to the original length | | `TestBitStreamUtil.TestPutVlqIntRoundTrip` | `PutVlqInt` round-trips every `uint32_t`, including values `>= 2^31` that used to hang | | `FaststringTest.TestReserve_ExponentiallyExpand` | `reserve()` does not reallocate on every call | Verified that the new tests actually catch the bug: built the exact repro (`RleEncoder<bool>::Put(true, 1 << 30)` followed by `Flush()`) against unpatched master and it never returns (killed by timeout); with this patch it completes and emits 8 bytes. All pre-existing tests in `rle_encoding_test`, `bit_stream_utils_test` and `faststring_test` pass (29 tests, with `LOOP_LESS_OR_MORE` forced to the slow/thorough branch). Also confirmed no format regression: a short run encoded via `Put(v)` in a loop and via `Put(v, n)` in one call still produce byte-identical output. - Behavior changed: - [x] No. Encoded output is unchanged for every run length that previously encoded correctly; only run lengths that used to hang now produce (correct, readable) output. - Does this need documentation? - [x] No. Co-authored-by: Xin Liao <[email protected]>
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
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: apache#65561
This is a stacked Draft PR. Its base branch,
feature-column-variant-v2, is the head branch of apache#65561. Review this PR as the incremental diff only.This PR adds:
variant_get(variant, constant_path), returning a Variant subvalue.variant_typeof(variant), currently returning a Spark-style recursive schema.variant_get + CAST, 32-field log-shaped objects, and recursive schema inference.Current scope and open design decision
These functions only accept compute-only, query-time Variant values, such as values produced by
parse_to_variantor supported casts. Persisted legacy Variant columns are not supported by this PR.variant_typeofis not compatible with the StarRocks function of the same name:ARRAY<BIGINT>andOBJECT<a: STRING>.ArrayorObject.The final function name and compatibility contract must be resolved before this PR is marked ready.
Benchmark interpretation
variant_get + CASTand StarRocks typed getters use the same 4,096-row inputs, constant paths, and result types. This is a user-visible expression comparison; Doris materializes an intermediate Nullable Variant while StarRocks uses a fused getter.variant_typeofcannot be compared with StarRocksvariant_typeofbecause they perform different work.Validation
BUILD_TYPE=Release ./build.sh --be— passedBUILD_TYPE=Release ./build.sh --benchmark— passedvariant_get + CAST, and recursivevariant_typeofrepresentative cases each completed a one-iteration smoke run./run-fe-ut.sh --run org.apache.doris.nereids.trees.expressions.functions.scalar.VariantFunctionsTest— 7/7 passedBUILD_TYPE_UT=ASAN ./run-be-ut.sh --run --filter='FunctionVariantGetTest.*:FunctionVariantTypeofTest.*'— 16/16 passedbuild-support/clang-format.sh,build-support/check-format.sh, andgit diff --check— passedrun-clang-tidy.shwas attempted with the matching Release toolchain; analysis is blocked by pre-existing diagnostics inbe/src/core/types.handbe/src/util/jsonb_document.hvariant_get_typeofregression suite could not run because no local FE was listening on127.0.0.1:9030Documentation
apache/doris-website#4001 documents the base Variant behavior but does not cover these functions. A follow-up documentation change is required before this PR is ready.