Doris-Breakwater commented on issue #68074:
URL: https://github.com/apache/doris/issues/68074#issuecomment-5695792433

   Breakwater-GitHub-Analysis-Slot: slot_69a5adce867b
   
   ## Initial assessment
   
   This is a confirmed VARIANT JSON-serialization correctness bug in 4.1.3, not 
a type-inference problem and not merely a formatting preference. The stored 
subcolumn can remain `boolean`, but the reconstructed JSON changes the JSON 
type from boolean to number. There is no evidence of on-disk type corruption; 
the loss occurs when VARIANT values are rendered as JSON text. Because that 
text can be consumed by JSON functions and external clients, the result is 
semantically incorrect and should be backported to 4.1 after a fix is validated.
   
   The issue is currently unlabelled. Please add the repository's normal bug 
and VARIANT/component labels.
   
   ## Verified code-path evidence
   
   - Rendering a non-scalar `VARIANT` as a string reaches 
`CastToStringFunction`, then `DataTypeVariantSerDe::to_string()`, which 
delegates to `ColumnVariant::serialize_one_row_to_string()` ([cast 
path](https://github.com/apache/doris/blob/4.1.3/be/src/exprs/function/cast/cast_to_string.h#L555-L580),
 [VARIANT 
SerDe](https://github.com/apache/doris/blob/4.1.3/be/src/core/data_type_serde/data_type_variant_serde.cpp#L172-L175)).
   - A dense VARIANT subcolumn is rendered by 
`ColumnVariant::Subcolumn::serialize_text_json()`, which delegates to the 
inferred subcolumn type's SerDe ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/column/column_variant.cpp#L1399-L1426)).
 For `TYPE_BOOLEAN`, `DataTypeNumberSerDe` enters the shared 
`is_int_or_bool(T)` branch and calls `write_number()` on the underlying 
`UInt8`, producing `1` or `0` ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/data_type_serde/data_type_number_serde.cpp#L204-L220)).
 The locally available current `branch-4.1` snapshot still has the same logic, 
so this is not already fixed there.
   - `ARRAY<BOOLEAN>` recurses through `DataTypeArraySerDe` into the same 
nested boolean SerDe, which explains `[1, 0]` ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/data_type_serde/data_type_array_serde.cpp#L46-L69)).
   - Sparse values preserve their binary type, but reconstruction decodes each 
value into a temporary `Subcolumn` and calls the same `serialize_text_json()` 
path ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/column/column_variant.cpp#L2011-L2025)).
 This explains why crossing `variant_max_subcolumns_count` changes JSONB-backed 
values to `1`/`0` without requiring a separate sparse-column defect.
   - JSONB is the working comparison because `DataTypeJsonbSerDe` uses 
`JsonbToJson`, which emits JSON boolean tokens ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/data_type_serde/data_type_jsonb_serde.cpp#L64-L79)).
   - `CAST(j['b'] AS STRING)` has an additional bypass that must be covered by 
the fix. `element_at` returns a scalar `VARIANT`, and 
`cast_from_variant_impl()` treats every scalar root as directly castable before 
its string-special case, so the Boolean root is cast with the ordinary SQL 
Boolean-to-string implementation (`1`/`0`) rather than the VARIANT JSON 
renderer ([variant 
cast](https://github.com/apache/doris/blob/4.1.3/be/src/exprs/function/cast/cast_to_variant.h#L68-L117),
 [Boolean string 
conversion](https://github.com/apache/doris/blob/4.1.3/be/src/exprs/function/cast/cast_to_string.h#L174-L190)).
 Fixing only structured-document serialization would therefore leave this 
reported expression unchanged.
   - `FormatOptions` already documents an `is_bool_value_num` switch whose 
false form is intended to render `[true]`, but the number SerDe currently does 
not consult it ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/core/data_type_serde/data_type_serde.h#L187-L194)).
 This is strong evidence for the missing formatting distinction, although the 
exact patch scope should be reviewed for compatibility.
   
   The distinction from #68016 is correct. A boolean-only path returns from the 
single-type case before mixed numeric supertype selection; changing 
boolean-plus-number merging cannot affect this serializer path.
   
   I did not rerun a Doris cluster locally, but the supplied reproduction is 
complete and deterministic, and the 4.1.3 source path directly accounts for the 
dense scalar, nested, array, JSONB, and sparse results.
   
   ## Missing information / decision needed
   
   No additional logs, profile, or reproduction details are needed to triage 
this issue. The only design decision needed before implementation is scope: 
JSON reconstruction for VARIANT must request JSON boolean literals while 
existing non-VARIANT display behavior for `BOOLEAN`, `CAST(BOOLEAN AS STRING)`, 
or `ARRAY<BOOLEAN>` should not be changed accidentally. In particular, globally 
changing the default boolean text format could create unrelated compatibility 
changes.
   
   ## Recommended next steps
   
   1. Add a failing VARIANT regression test covering dense `true`/`false`, 
nested booleans, `ARRAY<BOOLEAN>`, `CAST(j['b'] AS STRING)`, and a 
heterogeneous JSONB control case.
   2. Add a sparse-path case using a deliberately small 
`default_variant_max_subcolumns_count` so the test does not require 2,049 keys. 
Assert `true`, `false`, and `[true]` on both sides of the dense/sparse boundary.
   3. Implement the document-formatting distinction narrowly: make the Boolean 
number SerDe honor `FormatOptions::is_bool_value_num`, and ensure structured 
VARIANT JSON reconstruction passes the JSON-literal setting through dense, 
nested-array, document-value, and sparse paths.
   4. Handle scalar `VARIANT`-to-string separately so it retains VARIANT/JSON 
semantics instead of falling through to ordinary SQL Boolean-to-string 
conversion. Preserve the existing default for unrelated SQL/MySQL/collection 
rendering unless maintainers explicitly choose a broader behavior change. Also 
audit the string-root extraction branch, which currently inserts `1`/`0` 
explicitly for a parsed JSON boolean ([4.1.3 
source](https://github.com/apache/doris/blob/4.1.3/be/src/exprs/function/function_variant_element.cpp#L377-L388)).
   5. Run the relevant BE SerDe/ColumnVariant/cast unit tests and the VARIANT 
regression suites, including the combined matrix with #68016. Then backport the 
validated fix to branch-4.1.
   


-- 
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]

Reply via email to