eldenmoon opened a new pull request, #68016:
URL: https://github.com/apache/doris/pull/68016
### What problem does this PR solve?
Issue Number: None
Related PR: #67675
Problem Summary:
Within a single Variant V2 segment, when a path's first value is a JSON bool
and a plain number
(TINYINT/SMALLINT/INT/BIGINT/LARGEINT/FLOAT/DOUBLE) arrives afterwards, the
path silently turned the
stored `true`/`false` into the integer `1`/`0`. Canonical Variant/JSON
semantics require `true` to
never compare or group equal to `1`, so this changed GROUP BY / equality /
display results depending
on row order.
Root cause: `path_least_common_type()`
(be/src/storage/segment/variant/v2/variant_path_builder.cpp)
delegates ordinary scalar promotion to `get_least_supertype_jsonb()` /
`get_numeric_type()` (be/src/core/data_type/get_least_supertype.cpp:62-63),
which counts
`TYPE_BOOLEAN` as an 8-bit unsigned integer so it can share width promotion
with the real integer
types. That rule is correct for `get_numeric_type()`'s other callers
(data_type_array_serde.cpp,
nested_group_streaming_write_plan.cpp, variant_util.cpp), but for a Variant
path it merges BOOL and
a number into one numeric column instead of falling back to JSONB.
`promote()` then casts the
already-written `true` value to that numeric type, turning it into `1` in
storage. The opposite
order (a number first, then a bool) already fell back to JSONB and kept the
bool correct, because
`append_integer()` throws when a BOOL value hits an already-numeric
typed-path column and the
`catch` block in `VariantPathBuilder::append()` promotes to JSONB; only
bool-first was affected.
Reproduction (before the fix):
- BE UT (new):
`VariantPathBuilderTest.BoolFirstThenNumericFallsBackToJsonbPreservingBooleanValue`
failed with the path staying a numeric type and `to_string()` returning
`"1"` instead of `"true"`.
`BoolIntDoubleFalseSequenceKeepsBooleansAndNumbersDistinct` and
`BoolAndIntArraysFallBackToJsonbElementPreservingBooleanValue`
(ARRAY<BOOL> vs ARRAY<INT>) failed
the same way.
- SQL: a 1-bucket DUPLICATE table with one INSERT putting `{"k": true}`
before `{"k": 1}` read back
`var['k']` as `1` for the `true` row, and `GROUP BY var['k']` collapsed
the two logically distinct
rows into one group of size 2 instead of two groups of size 1.
Fix: in `path_least_common_type()`, when exactly one side is `TYPE_BOOLEAN`
and the other is one of
TINYINT/SMALLINT/INT/BIGINT/LARGEINT/FLOAT/DOUBLE, return the JSONB type
directly instead of
delegating to the shared numeric-tower helper (mirroring the existing
DECIMAL/ARRAY special-casing
in the same function). The shared `get_least_supertype.cpp` numeric rule is
left untouched since it
has other callers that legitimately want BOOLEAN folded into the numeric
tower; only this Variant
path type-inference site is changed. Arrays are covered automatically
because ARRAY-vs-ARRAY merges
already recurse into this function for the element type.
After the fix: all reproduction cases above pass; `var['k']` for the `true`
row reads back `true`,
and `GROUP BY` produces the correct distinct groups.
### Release note
Fixed a Variant V2 storage bug where a JSON boolean value could be silently
stored as the integer
1/0 when, within one segment, the same path saw a bool value before a
numeric value.
### Check List (For Author)
- Test:
- Unit Test: added
`VariantPathBuilderTest.BoolFirstThenNumericFallsBackToJsonbPreservingBooleanValue`,
`BoolIntDoubleFalseSequenceKeepsBooleansAndNumbersDistinct`, and
`BoolAndIntArraysFallBackToJsonbElementPreservingBooleanValue` in
be/test/storage/variant/variant_column_writer_reader_test.cpp. Ran the
full test file
(VariantPathBuilderTest, VariantShredderTest,
VariantColumnWriterReaderTest,
VariantWriterCompatibilityTest,
VariantSpecializedWriterCompatibilityTest): before the fix the
3 new cases failed as described above and all other cases passed;
after the fix all 91 run
cases passed (2 pre-existing skips: "NestedGroup write path is not
available in this build",
unrelated to this change).
- Regression test: added
regression-test/suites/variant_p0/test_variant_bool_numeric_widening.groovy,
generated its .out with -genOut and verified every row by reasoning.
Also ran the full
variant_p0 directory (173 suites) on the fixed build: 9 unrelated
pre-existing failures (1
missing S3/OSS credential for an outfile export test; 8
"debug_point/remove ... HTTP 500"
failures confirmed via direct API probe to be caused by
`config::enable_debug_points` being
disabled in this cluster, affecting only inverted-index debug-point
suites), none touching
Variant type inference; the other 164 suites passed.
- Behavior changed: Yes - a Variant path whose first value is a JSON bool
now falls back to JSONB
(instead of an incorrect numeric type) when a later value in the same path
is a plain number, so
`true`/`false` are preserved instead of becoming `1`/`0`.
- Does this need documentation: No
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
