github-actions[bot] commented on code in PR #66819:
URL: https://github.com/apache/doris/pull/66819#discussion_r3800872214
##########
be/src/format_v2/table/adbc_reader.cpp:
##########
@@ -647,6 +656,14 @@ Status AdbcFileReader::_materialize_arrow_column(const
std::string& column_name,
auto& columns = columns_guard.mutable_columns();
const auto& target_type =
columns_guard.get_datatype_by_position(block_position.value());
+ // The cached FE target remains authoritative when a source schema changes
mid-cache-window;
Review Comment:
[P1] Validate the recursive runtime type before SerDe
The cached FE target and runtime Arrow batch are matched only by the
top-level column name. If an `ARRAY<INT>` source changes to `ListView<Float32>`
during the cache window, canonicalization produces `List<Float32>` and this
guard passes; the target Int32 SerDe then takes its generic fixed-width path
and reinterprets the float buffer (`1.0f` becomes `1065353216`) instead of
rejecting the mismatch. The same missing boundary lets reordered same-typed
Struct children be mapped by ordinal. Please validate recursive child types and
normalized field identity against the cached target before mutating the block,
and add ListView/LargeListView schema-drift tests.
##########
be/src/format_v2/table/adbc_reader.cpp:
##########
@@ -608,7 +617,7 @@ Status AdbcFileReader::_materialize_record_batch(const
arrow::RecordBatch& batch
std::shared_ptr<arrow::Array> array;
{
SCOPED_TIMER(_normalize_time);
- RETURN_IF_ERROR(normalize_arrow_array(batch.column(arrow_idx),
&array));
+ RETURN_IF_ERROR(normalize_arrow_array(batch.column(arrow_idx),
arrow_pool, &array));
Review Comment:
[P1] Honor the mapper's nested projection
ADBC exposes synthesized complex children and inherits the default
`TableColumnMapper`, so a query may legitimately produce a partial
`LocalColumnIndex`. `TableReader` then builds the file-block type from that
projected subtree, but this loop checks only the root id and passes the
complete normalized Arrow root to SerDe. For `list_view<struct<a,b>>` projected
to only `b`, the target is `ARRAY<STRUCT<b>>` while Arrow still contains
`{a,b}`; in release builds `DataTypeStructSerDe` iterates the one target child
and reads source `field(0)`, placing `a` into `b` (the arity check is only a
`DCHECK`). Either force full complex scan projection with
`MaterializedColumnMapper` and let TableReader rematerialize, or
project/reorder the Arrow value from the request. Please add a
TableReader-level ListView Struct-subfield projection test.
##########
be/src/format/arrow/arrow_array_normalizer.cpp:
##########
@@ -89,14 +188,41 @@ Status normalize_arrow_array(const
std::shared_ptr<arrow::Array>& arr,
return Status::OK();
}
+ // List views may share or reorder value ranges, so rebuild canonical
offsets instead of
+ // exposing their buffers to a serde that requires contiguous list
values.
+ if (type.id() == arrow::Type::LIST_VIEW) {
+ auto converted = canonicalize_list_view<int32_t,
arrow::ListViewArray,
+ arrow::ListBuilder,
arrow::ListArray>(
+ static_cast<const arrow::ListViewArray&>(*current), pool);
Review Comment:
[P1] Normalize encoding wrappers below the rebuilt list
For `list_view<dictionary<int8,int8>>`, `MakeBuilder` creates a dictionary
child builder. Its `AppendArraySlice` inserts decoded values, but `Finish`
still emits a DictionaryArray backed by indices. The next normalizer pass sees
only the outer `LIST` as acceptable and returns it unchanged; ADBC maps the
dictionary's logical value type to `TINYINT`, so Array/Nullable/Number SerDes
copy the one-byte index buffer. Logical values `[42,43]` are therefore returned
as `[0,1]`, and default Arrow validation cannot distinguish the equally wide
buffers. Before this change the outer ListView was rejected. Please recursively
normalize or reject encoding-only descendants before accepting a complex outer
type, with ListView/LargeListView dictionary-child tests.
--
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]
