suxiaogang223 opened a new pull request, #69448:
URL: https://github.com/apache/doris/pull/69448
### What problem does this PR solve?
Issue Number: N/A
Related PR: #63001
Problem Summary:
A Parquet schema conversion can write past an empty destination null map and
crash the BE. For example, a TVF infers `BIGINT` from one file and then reads
`STRING` value `"xx"` from another file. `CastStringConverter` marks the failed
cast as NULL, but `PhysicalToLogicalConverter` currently copies the source null
map only after that conversion returns.
Prepare the appended destination null-map slice before logical conversion.
This preserves source NULLs and the existing destination prefix, lets
converters mark additional failed casts, and retains COW isolation. The same
boundary also covers other logical conversions that produce NULL, such as
numeric overflow. Non-nullable conversion errors remain unchanged.
Coverage added:
- Converter unit tests for BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY, nullable and
non-nullable sources, invalid and valid strings, source NULLs, empty
destinations, successive append batches, COW aliases, non-nullable errors, and
numeric overflow.
- A self-contained S3 TVF crash regression that generates a BIGINT schema
seed plus 4,096 STRING rows containing invalid values, numeric strings and
NULLs, then materializes the column through the original Parquet reader with
multiple batches. This is a crash-only regression; exact values and null maps
are checked by the unit tests, so no golden output is added.
### Release note
Fix a BE crash when Parquet schema conversion produces NULL for an invalid
value.
### Check List (For Author)
- Test:
- [x] Unit tests added; not executed locally.
- [x] Regression test added; not executed locally.
- Passed: build hygiene, clang-format 16 formatting/check, `git diff
--check`, Groovy syntax compilation.
- `bash run-be-ut.sh --run --filter='ParquetColumnConvertTest.*' -j 4`
was stopped while downloading the apache-orc submodule. The local third-party
installation also lacks `libhs.a`; no test executable was produced.
- `bash run-regression-test.sh --run -d external_table_p0/tvf -s
test_parquet_invalid_string_cast` failed before executing any suite: Maven 3.10
rejects existing duplicate Gson declarations in the regression framework POM.
- `run-clang-tidy.sh` could not run because no compilation database was
generated. Runtime and sanitizer validation remain for CI.
- Behavior changed:
- [x] Yes. Invalid nullable conversions produce NULL without accessing
an unallocated null map.
- Does this need documentation?
- [x] No.
<details>
<summary>Self-review checkpoints</summary>
- Goal: prepares the null map before any logical-converter failure write;
the new unit cases reproduce that boundary. Runtime validation is pending.
- Scope: one conversion boundary changed, with focused unit and crash
regression tests.
- Concurrency and locks: no new shared state or locking; columns belong to
the reader, and destination aliases are detached using existing COW helpers.
- Lifecycle and static initialization: no new lifetime mechanism or static
state; source null maps remain independently owned.
- Configuration: no new configuration; the regression restores its two
changed session settings.
- Compatibility: no symbol, protocol or storage-format changes.
- Parallel paths: ORC already aligns destination nulls before logical
conversion; File Scanner V2 uses a separate implementation. Consistent Parquet
conversion paths are unchanged.
- Conditions: no new production conditional branch; existing nullable
handling is reused.
- Coverage: source and conversion NULLs, valid values, two physical
representations, successive batches, COW and non-nullable errors are covered;
runtime execution is explicitly unverified.
- Expected results: checked directly in converter unit tests; the end-to-end
test checks successful materialization without a BE crash.
- Observability: existing non-nullable conversion errors remain sufficient;
no additional per-row logging is needed.
- Transactions/persistence: not involved in the production change.
- Data writes/atomicity: no production write-path changes; the regression
writes only its unique temporary S3 prefix.
- FE/BE variables: none added.
- Performance: the existing null-map copy is moved before conversion,
retaining linear cost and avoiding a second merge pass.
- Other issues: no additional source-level issue found in the modified path;
CI compilation, UT/sanitizer and external regression execution remain required.
</details>
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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]
