github-actions[bot] commented on code in PR #66305:
URL: https://github.com/apache/doris/pull/66305#discussion_r3700002648
##########
be/src/format/arrow/arrow_row_batch.cpp:
##########
@@ -51,6 +51,11 @@ namespace doris {
Status convert_to_arrow_type(const DataTypePtr& origin_type,
std::shared_ptr<arrow::DataType>* result,
const std::string& timezone) {
+ if (is_raw_byte_agg_state(origin_type)) {
Review Comment:
[P1] Preserve serialized AggState nullability in the Arrow field
A legal `agg_state<max(int null)>` stores `max_state(NULL)` as an internally
nullable `FixedLengthObject`, even though the outer AggState is deliberately
non-nullable. This branch emits that state through a Binary builder, but the
Parquet schema still takes field nullability from `root()->is_nullable()`
(false), making the byte column required and leaving no definition level for
the internal null. Please derive Arrow field nullability from the AggState's
serialized type and cover a `max_state(NULL)` Parquet round trip.
##########
be/src/exprs/function/cast/cast_base.cpp:
##########
@@ -128,6 +129,40 @@ Status cast_from_string_to_generic(FunctionContext*
context, Block& block,
return Status::OK();
}
+Status cast_from_binary_to_agg_state(FunctionContext* context, Block& block,
+ const ColumnNumbers& arguments, uint32_t
result,
+ size_t input_rows_count, const
NullMap::value_type* null_map) {
+ const auto& source = *block.get_by_position(arguments[0]).column;
+ auto& result_column_with_type = block.get_by_position(result);
+ auto result_column = result_column_with_type.type->create_column();
+ result_column->reserve(input_rows_count);
+
+ const IColumn* nested_result = result_column.get();
+ if (const auto* nullable_result =
check_and_get_column<ColumnNullable>(nested_result);
+ nullable_result != nullptr) {
+ nested_result = &nullable_result->get_nested_column();
+ }
+ const auto* fixed_result =
check_and_get_column<ColumnFixedLengthObject>(nested_result);
+ for (size_t row = 0; row < input_rows_count; ++row) {
+ if (null_map != nullptr && null_map[row]) {
+ result_column->insert_default();
+ continue;
+ }
+
+ const auto value = source.get_data_at(row);
+ if (fixed_result != nullptr && value.size !=
fixed_result->item_size()) {
Review Comment:
[P1] Propagate nullable binary sources before checking state width
FileScanner makes the Parquet byte column `Nullable`, but the target
AggState's outer type is non-nullable, so `prepare_remove_nullable()` does not
pass this source null map here. For a null row, `ColumnNullable::get_data_at()`
therefore returns a zero-byte value and this check rejects it as malformed
before the internally nullable fixed-state destination can preserve the null.
Please unwrap/propagate source nulls for byte-to-AggState casts (while still
rejecting nulls for a non-nullable serialized target) and add a nullable
fixed-state load case.
--
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]
