morningman commented on code in PR #67476:
URL: https://github.com/apache/doris/pull/67476#discussion_r3925061892
##########
be/src/core/column/column_variant.cpp:
##########
@@ -1439,6 +1439,18 @@ size_t
ColumnVariant::Subcolumn::serialize_text_json(size_t n, BufferWritable& o
const auto& part_type_serde = data_serdes[i];
if (ind < part.size()) {
+ // A part whose own type is Nothing holds no value, exactly like
the untyped root
+ // handled at the top of this function. The two are not the same
check: an unfinalized
+ // subcolumn keeps one part per source range
(Subcolumn::insert_range_from appends a
+ // new part rather than rewriting the earlier ones), and a later
typed part promotes
+ // the column-level least common type, so the check above stops
covering the rows that
+ // still sit in an untyped part. Render those rows as an empty
JSON object here too,
+ // instead of letting the Nothing serde emit an empty string. See
#67367.
+ if (get_base_type_of_array(data_types[i])->get_primitive_type() ==
+ PrimitiveType::INVALID_TYPE) {
Review Comment:
Confirmed and fixed in c2d58789708 — this was a real regression, not a
hypothetical one.
`get_base_type_of_array()` strips every array layer, so
`Nullable(Array(Nullable(Nothing)))` reduced to `Nullable(Nothing)` and matched
the new early return. And that type is exactly what an empty array gets:
`FieldVisitorToScalarType` inserts nothing into `type_indexes` for `[]`, so
`get_least_supertype_jsonb()` over the empty set returns `Nothing`
(`get_least_supertype.cpp:258-261`), and with `num_dimensions == 1`
`create_array_of_type()` builds `Nullable(Array(Nullable(Nothing)))`
(`column_variant.cpp:85-97`).
The rule now keys off the part's own type, which is precisely "scalar
Nothing, zero dimensions": `DataTypeNullable::get_primitive_type()` forwards to
its nested type (`data_type_nullable.h:58`), `DataTypeNothing` gives
`INVALID_TYPE`, and `DataTypeArray` gives `TYPE_ARRAY`.
Added `ColumnVariantEmptyObjectTest.empty_array_part_is_not_an_empty_object`
for the mixed-copy shape you describe. Against the previous check it fails with
```
serialize_subcolumn_row(subcolumn, 0)
Which is: "{}" expected "[]"
part type Nullable(Array(Nullable(Nothing))), column type
Nullable(Array(Nullable(TINYINT)))
```
and it passes with this change. All 6 cases in the suite pass.
--
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]
