morningman commented on code in PR #67476:
URL: https://github.com/apache/doris/pull/67476#discussion_r3925064294
##########
be/src/exprs/function/cast/cast_to_variant.h:
##########
@@ -67,11 +69,32 @@ inline Status cast_from_variant_impl(FunctionContext*
context, Block& block,
// if the root of this variant column is a number column, converting it to
a number column
// is acceptable. However, if the destination type is a string and root is
none scalar root, then
// we should convert the entire tree to a string.
- bool is_root_valuable = variant->is_scalar_variant() ||
- (!variant->is_null_root() &&
- variant->get_root_type()->get_primitive_type() !=
INVALID_TYPE &&
-
!is_string_type(data_type_to->get_primitive_type()) &&
- data_type_to->get_primitive_type() != TYPE_JSONB);
+ // A scalar variant is only worth converting through its root when the
root actually carries a
+ // value. A column whose rows are all empty JSON objects is still "scalar"
- it simply has no
+ // path, so its root never got a type - and routing it through the root
conversion leaves
+ // nothing to convert and turns every row into NULL, instead of the `{}`
that serializing the
+ // whole tree produces for the very same value in a column that does hold
paths. See #67367.
+ const bool root_carries_value = !variant->is_null_root() &&
+
variant->get_root_type()->get_primitive_type() != INVALID_TYPE;
+ const bool to_string_or_jsonb =
is_string_type(data_type_to->get_primitive_type()) ||
+ data_type_to->get_primitive_type() ==
TYPE_JSONB;
+ bool is_root_valuable =
+ root_carries_value && (variant->is_scalar_variant() ||
!to_string_or_jsonb);
+ if (is_root_valuable && to_string_or_jsonb) {
+ // Once the root is typed, a row that holds an empty JSON object shows
up as a NULL root
+ // inside a variant row that is not itself NULL. Converting such a
root to STRING/JSONB
+ // yields NULL, while serializing the tree renders it as `{}` - which
is what `SELECT
+ // <variant>` returns, and what the same value returns from a column
that also holds
+ // paths. When no row's root holds a value there is nothing for the
root conversion to
+ // convert, so serialize the tree instead. Rows whose root does hold a
value keep the root
+ // conversion, which is what unwraps a JSON string into its text. See
#67367.
+ const auto* nullable_root =
check_and_get_column<ColumnNullable>(*variant->get_root());
+ if (nullable_root != nullptr) {
+ const auto& root_null_map = nullable_root->get_null_map_data();
+ is_root_valuable = std::any_of(root_null_map.begin(),
root_null_map.end(),
+ [](UInt8 is_null) { return is_null
== 0; });
Review Comment:
Scan tightened in c2d58789708: it is now bounded by `input_rows_count` and
skips the rows the outer null map masks out.
The outer-null-map half is real — `prepare_remove_nullable` unnests the
argument and hands `arg_null_map` over separately
(`function_cast.cpp:206-219`), and `need_replace_null_data_to_default` returns
false for STRING/JSONB targets, so the masked rows keep their payloads.
One correction for the record: it was not a regression from this PR. In your
`v = [{}, 123]` example the pre-PR code also returned NULL for the visible `{}`
row, because `is_root_valuable` was simply `is_scalar_variant()`. The new
`any_of` can only ever lower `is_root_valuable`, never raise it. So that case
is the mixed-root gap the PR description calls out as deliberately out of scope
— but honouring the mask closes this sub-case of it, so it is in.
On the prefix half, I could not find a live caller where `input_rows_count <
col_from.size()` for this path: `vcast_expr.cpp:122` and `:171` pass
`temp_block.rows()`, the per-row try_cast path at `:236` passes a 1-row `cut`
with count 1, `prepare_remove_nullable` forwards `input_rows_count` unchanged
over same-size unnested columns, and const arguments go through a temporary
block sized by its own `rows()`. The variant is also `clone_finalized()` before
this point, so the root's size equals the column's. Bounding the scan costs
nothing either way, so it is bounded.
--
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]
