morningman opened a new pull request, #69064:
URL: https://github.com/apache/doris/pull/69064
### What problem does this PR solve?
Issue Number: None
Related PR: #66012, #62821
Problem Summary:
**In short.** A broker load that reads a column from the file path (`COLUMNS
FROM PATH AS (p)`) stores a path value of `\N` as the two-character string `\N`
on master. Before #62821, and on branch-4.1 (#66012), the load stores NULL.
This PR ports the load-specific parsing of branch-4.1 #66012 so that loads
store NULL again; queries are not changed.
**Background.** For `COLUMNS FROM PATH AS`, the FE parses every file path
(for example `s3://bucket/t/p=\N/data.csv`) and sends the values to the BE in
the scan range, together with a null flag per value
(`columns_from_path_is_null`). #62821 added the flag and sets it only for the
Hive default partition, `__HIVE_DEFAULT_PARTITION__`. Before that, the FE wrote
a NULL as `\N` and the BE read the values with its nullable parser, so a
literal `\N` directory value became NULL as well. branch-4.1 got the same flag
in #66012, which also added a load-specific parser that keeps the old meaning
of `\N` for loads.
**The problem, and what it cost.**
| Path value | master | before #62821, and branch-4.1 | with this PR |
|---|---|---|---|
| `p=\N`, string column | the string `\N` | NULL | NULL |
| `p=\N`, INT column | NULL from the load's cast; the row is filtered in
strict mode | NULL | NULL |
| `p=__HIVE_DEFAULT_PARTITION__` | NULL | NULL | NULL |
| Queries (Hive catalog, TVF, `INSERT ... SELECT FROM s3()`) | `\N` is data
| `\N` is data | unchanged |
**How this PR fixes it.**
`FilePartitionUtils.parseColumnsFromPathWithNullInfoForLoad` parses the path as
before and then turns a literal `\N` into an empty value with the null flag
set, like branch-4.1's `BrokerUtil.parseColumnsFromPathWithNullInfoForLoad`.
Both range-building paths of `NereidsFileGroupInfo` (splittable and
unsplittable files) use it, and so does the legacy `FileGroupInfo`, which
master no longer instantiates. Stream load does not send path columns and is
not affected.
**Results.**
`BrokerUtilTest#parseColumnsFromPathForLoadKeepsLegacyNullSemantics` (from
branch-4.1, plus a case with a regular value and the Hive default partition)
checks the parser.
`NereidsFileGroupInfoTest#loadRangesCarryLiteralNullPathValueAsNull` builds the
scan ranges of a load for a splittable and an unsplittable CSV file under
`p=\N` and checks that the range carries an empty value with the null flag set.
Switching either call site, or the parser, back to the query behavior makes the
corresponding test fail. `BrokerUtilTest`, `NereidsFileGroupInfoTest` and
`NereidsLoadScanProviderTest` pass (12 tests). There is no regression test:
`test_load_data_from_path` reads fixed objects from the shared S3 test bucket,
which has no `p=\N` object.
Original author: @suxiaogang223 (#66012)
### Release note
A broker load that reads a column from a path value `\N` (`COLUMNS FROM PATH
AS`) stores NULL again instead of the string `\N`.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
-
`BrokerUtilTest#parseColumnsFromPathForLoadKeepsLegacyNullSemantics` and
`NereidsFileGroupInfoTest#loadRangesCarryLiteralNullPathValueAsNull` (new);
each fails when its call site or the parser uses the query behavior. 12 tests
in the three classes pass.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
- A literal `\N` path value is loaded as NULL, as before #62821 and
on branch-4.1.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
