Doris-Breakwater commented on issue #66039:
URL: https://github.com/apache/doris/issues/66039#issuecomment-5077595669
Breakwater-GitHub-Analysis-Slot: slot_48353ec1370c
## Initial triage
**Assessment: confirmed correctness bug in the Hive TEXTFILE deserialization
path.** The issue is actionable without a runtime profile or additional logs.
It is currently open with no labels, and the author has already opened #66040.
### Code-path confirmation
- On current `master`, both
`DataTypeArraySerDe::deserialize_one_cell_from_hive_text` and
`DataTypeMapSerDe::deserialize_one_cell_from_hive_text` return
`InvalidArgument` before appending a row when the input slice is empty.
- Both the legacy and v2 Hive text readers preserve an empty raw field and
call `deserialize_one_cell_from_hive_text`; missing trailing fields are
separately materialized using the configured `null_format`. This means an
actual empty field and a missing/explicit-NULL field are distinguishable at the
SerDe boundary.
- `DataTypeNullableSerDe::deserialize_one_cell_from_hive_text` checks the
configured NULL marker first. For any other input, it delegates to the nested
SerDe and converts any nested error to NULL. This proves the reported nullable
behavior: the ARRAY/MAP empty-slice error is silently converted to NULL.
- For a non-nullable ARRAY/MAP, there is no nullable wrapper to absorb the
error, so it propagates to the text reader.
- Hive's own lazy SerDe explicitly treats a zero-length ARRAY or MAP payload
as a zero-element collection: see `LazyArray.parse()` and `LazyMap.parse()` in
Apache Hive. This supports the requested semantic distinction between an empty
field and the configured NULL marker.
The same defect is present on `branch-4.0`, although the files are under the
pre-refactor paths `be/src/vec/data_types/serde/`. A master-only change will
therefore not fix 4.0.
### Fix assessment
The approach in #66040 is mechanically correct: appending the previous
offset and returning OK creates one collection row containing zero nested
elements. For a nullable top-level column, the nullable wrapper then appends a
`0` null-map bit, producing non-NULL `[]`/`{}`. The configured NULL marker is
still recognized by the nullable wrapper before delegation.
The change should remain limited to the Hive-text ARRAY/MAP methods. JSON
and generic string parsing require bracketed representations and should not be
changed, and leaving STRUCT out is appropriate.
One wording correction is needed in #66040: this patch affects Hive
TEXTFILE, not ordinary CSV. The CSV readers call
`deserialize_one_cell_from_csv`, whose ARRAY/MAP default is JSON
deserialization; they do not call the two methods changed by this patch.
There is also one configuration caveat worth documenting in the test or PR
description: if a Hive table explicitly configures the empty string itself as
`serialization.null.format`, an empty nullable field must still resolve to NULL
because the configured NULL marker is checked first. The empty collection
behavior applies when empty bytes and `null_format` are distinct, including the
default `\N`.
### Missing information
No additional diagnostic information is required to establish the root
cause. For a durable end-to-end regression, it would still be useful to include
the exact Hive DDL/SerDe properties and raw fixture rows, especially the
configured `serialization.null.format`; the current issue only gives a
conceptual reproduction.
### Recommended next steps
1. Review #66040 as the master fix. Its direct SerDe tests cover the primary
behavior, and the author reports six passing ASAN cases.
2. Add or extend an end-to-end Hive text regression with adjacent rows for
non-empty, empty, and `\N` values for both ARRAY and MAP. This verifies field
splitting plus SerDe behavior, not only direct method calls.
3. Strengthen the unit coverage with explicit `\N` for MAP and a non-empty →
empty → non-empty sequence to verify offset continuity. A custom empty
`null_format` case would lock down the precedence described above.
4. After master validation, prepare a separate 4.0 backport against the
`be/src/vec/data_types/serde/` implementation if that branch is supported for
this fix.
5. Apply the repository's existing bug/Hive/external-table/BE labels as
appropriate; the issue currently has no labels.
--
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]
