morningman opened a new pull request, #69567:
URL: https://github.com/apache/doris/pull/69567
### What problem does this PR solve?
Issue Number: None
Related PR: #68874
Problem Summary:
**In short.** This follows up #68874 on Paimon tables with a BINARY /
VARBINARY partition column. A write to such a table whose partitions use
Paimon's legacy names now fails while it is planned, with a hint to set
`'partition.legacy-name' = 'false'`, instead of failing its commit and leaving
files behind. On a table with the new names, `INSERT OVERWRITE ... PARTITION (p
= X'...')` is supported when the bytes are UTF-8 text; #68874 had to reject
every non-NULL binary static overwrite.
**Background.** Paimon names a partition directory after the string of each
partition value. With `partition.legacy-name = true`, the default, that string
is `toString()` of the value, which for a binary value is the Java array object
(`[B@1b6d3586`) rather than its bytes. With `partition.legacy-name = false` it
is the text of the bytes (`p=AB` for `X'4142'`). Doris cannot create such a
table itself (its DDL rejects VARBINARY columns), so these tables come from
Spark, Flink or the Paimon API. Paimon's static overwrite
(`withOverwrite(Map<String, String>)`) names the partition to replace by a
string as well, and takes the UTF-8 bytes of that string as a binary value.
**The problem, and what it cost.** Measured on master with Paimon 2.1.0, on
a `file://` Paimon catalog of a local FE + BE, with tables created through the
Paimon API:
| Table / statement | Paimon API | Doris before this PR |
|---|---|---|
| `legacy-name = true`, write `X'4142'`, then read | the write succeeds, the
read fails with `FileNotFoundException` (every resolution of the name gives
another `[B@...`) | `INSERT` fails at commit: `Cannot recover from this
checkpoint because some files in the snapshot that need to be resubmitted have
been deleted`, and the BE's files stay under `p=%5BB@...` |
| `legacy-name = false`, `INSERT` of `X'4142'`, `X'EF'`, NULL (also
`BINARY(4)`, and a multi-instance `INSERT ... SELECT`) | writes and reads |
writes and reads correctly |
| `legacy-name = false`, `INSERT OVERWRITE ... PARTITION (p = X'4142')
SELECT ...` | — | rejected: `Static overwrite of binary partition column 'p' is
not supported` |
The rejection came from #68874: the cast value of a binary static partition
reaches the connector as `0x` and its hex digits, and Paimon would take the six
bytes of `"0x4142"`, naming another partition; an overwrite without rows then
reported success and left the given partition in place. The legacy failure
costs a failed statement whose files nobody cleans up, on a table that Paimon
itself cannot read back.
**How this PR fixes it.**
- `PaimonWriteBinding.create`, which every Paimon write (INSERT, OVERWRITE,
row-level DML) goes through while it is planned, rejects a table that has a
binary partition column and legacy partition names, naming the table option to
set.
- `PaimonWriteBinding.resolveStaticPartition` decodes the hex digits of a
binary static value and passes Paimon the text the bytes decode to as UTF-8,
which Paimon encodes back to the same bytes. The decoder is strict, so bytes
that are not UTF-8 text (a lone `0xEF`, an overlong or surrogate sequence),
which no string names, are still rejected, with the value in the message. A
NULL static value keeps naming the `partition.default-name` partition, and a
text equal to that name is still rejected, as for other types.
**Results.** On the same local cluster, with a `legacy-name = false` table
holding `(1, X'4142')`, `(2, X'4344')`, `(3, X'EF')`, `(4, NULL)`, `(6, X'')`:
| Statement | After this PR (Doris read = Paimon API read) |
|---|---|
| `INSERT OVERWRITE ... PARTITION (p = X'4142') SELECT 5` | the `X'4142'`
partition holds only 5 |
| `INSERT OVERWRITE ... PARTITION (p = X'4344') SELECT 1 LIMIT 0` | the
`X'4344'` partition is empty, the others are untouched |
| `INSERT OVERWRITE ... PARTITION (p = X'EF') SELECT 1` | fails: `Static
partition value 0xEF of binary column 'p' is not UTF-8 text, ...`; 3 stays |
| `INSERT OVERWRITE ... PARTITION (p = NULL) SELECT 9` | the NULL partition
holds only 9 |
| `INSERT OVERWRITE ... PARTITION (p = X'') SELECT 7` | the empty-bytes
partition holds only 7; the NULL partition keeps 9 |
| `INSERT` / `INSERT OVERWRITE` into a `legacy-name = true` table | fails
while planning: `... its binary partition column 'p' needs the table option
'partition.legacy-name' = 'false' ...`; no files are written |
`PaimonWriteBindingTest` replaces the test of the #68874 rejection with
three tests: a binary static value becomes the text of its bytes and Paimon
parses it back to them (for `BYTES`, `VARBINARY(2)`, `BINARY(2)`), bytes that
are not UTF-8 are rejected, and a write of a binary partition with legacy names
fails while one with new names, or a legacy table without a binary partition,
is planned. They fail when the legacy check is dropped, when the cast value is
passed through, and when the decoding replaces malformed bytes. All 847 tests
of fe-connector-paimon pass.
No regression test is added: Doris cannot create a binary column, so the
table would have to come from the Spark container of the Paimon docker
environment, which was not run locally.
### Release note
Fix: a write to a Paimon table whose binary partition column uses legacy
partition names fails at planning with a hint to set `'partition.legacy-name' =
'false'`, instead of failing its commit and leaving files behind. `INSERT
OVERWRITE ... PARTITION (p = X'...')` on a binary partition column of a Paimon
table is supported when the bytes are UTF-8 text.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
Local FE + BE with `CREATE CATALOG ... PROPERTIES ('type' = 'paimon',
'warehouse' = 'file:///...')`; tables created with the Paimon Catalog API
(`Schema.newBuilder().column("p", DataTypes.BYTES()).partitionKeys("p")`, with
and without `partition.legacy-name = false`); the statements in the tables
above, read back through Doris and through the Paimon API.
- [ ] 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 write to a Paimon table with a binary partition column and legacy
partition names fails at planning instead of at commit. A static overwrite of a
binary partition column with a binary literal whose bytes are UTF-8 text is
accepted instead of rejected.
- 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]
