github-actions[bot] commented on code in PR #68667:
URL: https://github.com/apache/doris/pull/68667#discussion_r4144002446
##########
be/src/core/data_type_serde/data_type_variant_serde.cpp:
##########
@@ -28,19 +30,141 @@
#include "common/status.h"
#include "core/assert_cast.h"
#include "core/column/column.h"
+#include "core/column/column_array.h"
+#include "core/column/column_map.h"
+#include "core/column/column_struct.h"
#include "core/column/column_variant.h"
+#include "core/column/variant_v2/column_variant_v2.h"
+#include "core/column/variant_v2/column_variant_v2_typed_column.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_nullable.h"
+#include "core/data_type/data_type_struct.h"
#include "core/data_type_serde/data_type_serde.h"
+#include "core/data_type_serde/data_type_variant_v2_serde.h"
#include "core/field.h"
#include "core/string_ref.h"
#include "core/types.h"
#include "core/value/jsonb_value.h"
#include "exec/common/variant_util.h"
+#include "exprs/function/parse/variant_jsonb_parse.h"
+#include "exprs/function/parse/variant_string_parse.h"
#include "util/json/json_parser.h"
#include "util/jsonb_writer.h"
namespace doris {
namespace {
+// Legacy CAST accepts more root families than V2 CAST. Encode their structure
here so
+// Flight output does not reject valid roots or lose typed leaves through JSON
reparsing.
+Status append_legacy_arrow_value(const IColumn& column, const DataTypePtr&
type, size_t index,
+ VariantBatchBuilder::Row& output,
+ const DataTypeSerDe::FormatOptions& options,
size_t depth = 0) {
+ if (depth > VARIANT_MAX_NESTING_DEPTH) {
+ return Status::NotSupported(
+ "Native Arrow Variant nesting exceeds {}; "
+ "use enable_arrow_flight_sql_native_variant=false for UTF8
output",
+ VARIANT_MAX_NESTING_DEPTH);
+ }
+ if (const auto* constant = check_and_get_column<ColumnConst>(column)) {
+ return append_legacy_arrow_value(constant->get_data_column(), type, 0,
output, options,
+ depth);
+ }
+ if (const auto* nullable = check_and_get_column<ColumnNullable>(column)) {
+ if (nullable->is_null_at(index)) {
+ output.add_null();
+ return Status::OK();
+ }
+ return append_legacy_arrow_value(nullable->get_nested_column(),
remove_nullable(type),
+ index, output, options, depth);
+ }
+ const auto primitive = type->get_primitive_type();
+ if (is_supported_variant_typed_identity(primitive)) {
+ dispatch_variant_typed_column(
+ column, primitive, [&]<PrimitiveType Type>(const auto& scalar)
{
+ with_variant_typed_scalar<Type>(
+ scalar, index,
cast_set<uint8_t>(type->get_scale()),
+ [&](const VariantScalarRef& value) {
output.add_scalar(value); });
+ });
+ } else if (primitive == TYPE_TIMEV2) {
+ // TIMEV2 already stores microseconds; treating its physical double as
a number loses its type.
+ const double micros = assert_cast<const
ColumnTimeV2&>(column).get_data()[index];
+ if (!std::isfinite(micros) ||
+ std::abs(micros) >=
static_cast<double>(std::numeric_limits<int64_t>::max())) {
+ return Status::InvalidArgument("Invalid native Arrow Variant
TIMEV2 value");
+ }
+ output.add_time_ntz_micros(std::llround(micros));
Review Comment:
[P2] Keep duration values outside one day out of Variant TIME_NTZ_MICROS.
Doris accepts `-01:00:00` and `25:00:00` as TIMEV2, but this writes their
signed microseconds as a Parquet Variant time-of-day value, defined as
microseconds after midnight. Such values are invalid for conforming decoders.
Encode them in a representable form or return an explicit native-mode
compatibility error; test negative and over-24-hour roots and nested leaves.
[Parquet Variant type
mapping](https://github.com/apache/parquet-format/blob/master/VariantEncoding.md)
##########
be/src/core/data_type_serde/data_type_variant_serde.cpp:
##########
@@ -28,19 +30,141 @@
#include "common/status.h"
#include "core/assert_cast.h"
#include "core/column/column.h"
+#include "core/column/column_array.h"
+#include "core/column/column_map.h"
+#include "core/column/column_struct.h"
#include "core/column/column_variant.h"
+#include "core/column/variant_v2/column_variant_v2.h"
+#include "core/column/variant_v2/column_variant_v2_typed_column.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_nullable.h"
+#include "core/data_type/data_type_struct.h"
#include "core/data_type_serde/data_type_serde.h"
+#include "core/data_type_serde/data_type_variant_v2_serde.h"
#include "core/field.h"
#include "core/string_ref.h"
#include "core/types.h"
#include "core/value/jsonb_value.h"
#include "exec/common/variant_util.h"
+#include "exprs/function/parse/variant_jsonb_parse.h"
+#include "exprs/function/parse/variant_string_parse.h"
#include "util/json/json_parser.h"
#include "util/jsonb_writer.h"
namespace doris {
namespace {
+// Legacy CAST accepts more root families than V2 CAST. Encode their structure
here so
+// Flight output does not reject valid roots or lose typed leaves through JSON
reparsing.
+Status append_legacy_arrow_value(const IColumn& column, const DataTypePtr&
type, size_t index,
+ VariantBatchBuilder::Row& output,
+ const DataTypeSerDe::FormatOptions& options,
size_t depth = 0) {
+ if (depth > VARIANT_MAX_NESTING_DEPTH) {
+ return Status::NotSupported(
+ "Native Arrow Variant nesting exceeds {}; "
+ "use enable_arrow_flight_sql_native_variant=false for UTF8
output",
+ VARIANT_MAX_NESTING_DEPTH);
+ }
+ if (const auto* constant = check_and_get_column<ColumnConst>(column)) {
+ return append_legacy_arrow_value(constant->get_data_column(), type, 0,
output, options,
+ depth);
+ }
+ if (const auto* nullable = check_and_get_column<ColumnNullable>(column)) {
+ if (nullable->is_null_at(index)) {
+ output.add_null();
+ return Status::OK();
+ }
+ return append_legacy_arrow_value(nullable->get_nested_column(),
remove_nullable(type),
+ index, output, options, depth);
+ }
+ const auto primitive = type->get_primitive_type();
+ if (is_supported_variant_typed_identity(primitive)) {
+ dispatch_variant_typed_column(
+ column, primitive, [&]<PrimitiveType Type>(const auto& scalar)
{
+ with_variant_typed_scalar<Type>(
+ scalar, index,
cast_set<uint8_t>(type->get_scale()),
+ [&](const VariantScalarRef& value) {
output.add_scalar(value); });
+ });
+ } else if (primitive == TYPE_TIMEV2) {
+ // TIMEV2 already stores microseconds; treating its physical double as
a number loses its type.
+ const double micros = assert_cast<const
ColumnTimeV2&>(column).get_data()[index];
+ if (!std::isfinite(micros) ||
+ std::abs(micros) >=
static_cast<double>(std::numeric_limits<int64_t>::max())) {
+ return Status::InvalidArgument("Invalid native Arrow Variant
TIMEV2 value");
+ }
+ output.add_time_ntz_micros(std::llround(micros));
+ } else if (primitive == TYPE_JSONB) {
+ jsonb_to_variant(column.get_data_at(index), output);
+ } else if (primitive == TYPE_ARRAY) {
+ const auto& array = assert_cast<const ColumnArray&>(column);
+ const auto& array_type = assert_cast<const DataTypeArray&>(*type);
+ auto scope = output.start_array();
+ for (size_t element = array.offset_at(index); element <
array.get_offsets()[index];
+ ++element) {
+ RETURN_IF_ERROR(append_legacy_arrow_value(array.get_data(),
+
array_type.get_nested_type(), element, output,
+ options, depth + 1));
+ }
+ scope.finish();
+ } else if (primitive == TYPE_MAP) {
+ const auto& map = assert_cast<const ColumnMap&>(column);
+ const auto& map_type = assert_cast<const DataTypeMap&>(*type);
+ auto scope = output.start_object();
+ for (size_t element = map.get_offsets()[static_cast<ssize_t>(index) -
1];
+ element < map.get_offsets()[index]; ++element) {
+ // Variant objects have textual keys, matching the legacy document
representation.
+ auto key =
+ map.get_keys().is_null_at(element)
+ ? std::string("null")
Review Comment:
[P2] Preserve distinct MAP keys in native Variant encoding. `MAP(CAST(NULL
AS STRING), 1, 'null', 2)` keeps two keys in Doris, but this branch turns SQL
NULL into `null`, the same object name produced for the string key.
`VariantBatchBuilder` then rejects the second key and native Flight fails,
while UTF8 output can return the legacy value. Use a collision-safe
representation or give an actionable unsupported-value error, and cover root
and nested MAPs with these keys.
--
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]
