Gabriel39 commented on code in PR #68667:
URL: https://github.com/apache/doris/pull/68667#discussion_r4142110678
##########
be/src/core/data_type_serde/data_type_variant_serde.cpp:
##########
@@ -157,6 +165,101 @@ Status DataTypeVariantSerDe::write_column_to_arrow(const
IColumn& column, const
int64_t start, int64_t end,
const cctz::time_zone& ctz)
const {
const auto* var = check_and_get_column<ColumnVariant>(column);
+ if (array_builder->type()->id() == arrow::Type::STRUCT) {
+ // Legacy documents need JSON conversion; typed scalar roots can keep
their type.
+ // The outer null map must remain SQL NULL on the wire.
+ if (start < 0 || end < start || end > column.size() ||
+ (null_map != nullptr && end > null_map->size())) {
+ return Status::InvalidArgument("Invalid Variant Arrow row range
[{}, {})", start, end);
+ }
+ if (var->is_scalar_variant()) {
+ auto scalar_type = remove_nullable(var->get_root_type());
+ if (scalar_type->get_primitive_type() == TYPE_DECIMAL256) {
+ return Status::NotSupported(
+ "Native Arrow Variant does not support Decimal256
roots");
+ }
+ if
(is_supported_variant_typed_identity(scalar_type->get_primitive_type())) {
+ // Avoid a JSON round trip that would turn exact decimal roots
into doubles.
+ auto typed =
+
ColumnVariantV2::create_typed(make_nullable(var->get_root()), scalar_type);
+ return DataTypeVariantV2SerDe().write_column_to_arrow(
+ *typed, null_map, array_builder, start, end, ctz);
+ }
+ }
+ const size_t rows = end - start;
+ NullMap selected_nulls(rows, 0);
+ NullMap root_mask(rows, 1);
+ bool has_roots = false;
+ bool has_documents = false;
+ for (size_t row = 0; row < rows; ++row) {
+ selected_nulls[row] = null_map != nullptr && (*null_map)[start +
row];
+ if (selected_nulls[row]) {
+ continue;
+ }
+ const bool root_visible = var->is_scalar_variant()
+ ?
!var->get_root()->is_null_at(start + row)
+ :
var->is_visible_root_value(start + row);
+ root_mask[row] = !root_visible;
+ has_roots |= root_visible;
+ has_documents |= !root_visible;
+ }
+ ColumnPtr roots;
+ if (has_roots) {
+ // JSON reparsing loses decimal/temporal identities and rejects
non-finite numbers.
+ // Reuse typed CAST for every visible root, including arrays and
mixed-path batches.
+ auto root_type = remove_nullable(var->get_root_type());
+ auto target_type = std::make_shared<DataTypeVariantV2>();
+ // Invisible roots, including nullable roots, are already masked
above.
+ Block root_block {
+ {remove_nullable(var->get_root())->cut(start, rows),
root_type, "root"},
+ {target_type->create_column(), target_type, "encoded"}};
+ auto encode_root =
CastWrapper::create_cast_to_variant_v2_wrapper(root_type);
+ RETURN_IF_ERROR(encode_root(nullptr, root_block, {0}, 1, rows,
root_mask.data()));
Review Comment:
Fixed in 474438bfe2d. Flight now recursively encodes visible legacy roots
instead of delegating them to the narrower V2 CAST interface. MAP and STRUCT
retain their structure and exact typed leaves; ARRAY traverses nested
containers and legacy/V2 VARIANT leaves; TIMEV2 uses the native microsecond
time primitive. Added root, one-level-array and two-level-array cases,
including both textual and integer MAP keys and exact decimal fields. The new
cases reproduced the old rejection before the fix. All 39 related BE tests and
the ADBC direct/partition transport checks pass.
##########
be/src/core/data_type_serde/data_type_variant_serde.cpp:
##########
@@ -157,6 +165,101 @@ Status DataTypeVariantSerDe::write_column_to_arrow(const
IColumn& column, const
int64_t start, int64_t end,
const cctz::time_zone& ctz)
const {
const auto* var = check_and_get_column<ColumnVariant>(column);
+ if (array_builder->type()->id() == arrow::Type::STRUCT) {
+ // Legacy documents need JSON conversion; typed scalar roots can keep
their type.
+ // The outer null map must remain SQL NULL on the wire.
+ if (start < 0 || end < start || end > column.size() ||
+ (null_map != nullptr && end > null_map->size())) {
+ return Status::InvalidArgument("Invalid Variant Arrow row range
[{}, {})", start, end);
+ }
+ if (var->is_scalar_variant()) {
+ auto scalar_type = remove_nullable(var->get_root_type());
+ if (scalar_type->get_primitive_type() == TYPE_DECIMAL256) {
+ return Status::NotSupported(
+ "Native Arrow Variant does not support Decimal256
roots");
+ }
+ if
(is_supported_variant_typed_identity(scalar_type->get_primitive_type())) {
+ // Avoid a JSON round trip that would turn exact decimal roots
into doubles.
+ auto typed =
+
ColumnVariantV2::create_typed(make_nullable(var->get_root()), scalar_type);
Review Comment:
Fixed in 474438bfe2d. The scalar shortcut is now used only when the
requested root range contains no inner nulls. Otherwise the existing
root-visibility/document path preserves the legacy empty object while the outer
null map still yields SQL NULL. Added a scalar-only batch containing 42, a
legacy null root, and SQL NULL, with full-range and sliced assertions; this
test failed on the previous commit and passes after the fix. The documented
legacy null semantics have also been clarified.
--
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]
