morningman opened a new pull request, #69571:
URL: https://github.com/apache/doris/pull/69571
### What problem does this PR solve?
Issue Number: None
Related PR: #69567
Problem Summary:
**In short.** A query of a Paimon table with a BINARY / VARBINARY partition
column now fails with an error naming `'partition.legacy-name'` when it would
read a non-NULL binary partition whose files Paimon cannot locate, instead of
silently returning only the rows of the other partitions. This is the read-side
counterpart of #69567, which rejects writes to such tables.
**Background.** Paimon looks for the files of a partition under the
directory its partition values name, computed when the split is planned; a
writer puts the files under the name it computes when it writes. With
`partition.legacy-name = true`, the default, the name of a value is its
`toString()`, which for a binary value is the Java array object (`[B@1b6d3586`)
rather than its bytes, so every resolution differs. With `partition.legacy-name
= false` it is the text of the bytes (`p=AB` for `X'4142'`). The option can be
changed on an existing table, which changes how existing partitions are looked
up but does not move their files. The BE skips a file of an external table that
no longer exists (`ignore_not_found_file_in_external_table`, default true),
which is meant for files removed between planning and reading.
**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 and written
through the Paimon API:
| Table | Paimon API read | Doris before this PR |
|---|---|---|
| legacy names (default), rows in `X'4142'`, `X'EF'`, NULL | fails:
`FileNotFoundException` (`p=%5BB@...`) | `SELECT *` returns only the NULL row;
`COUNT(*)`, answered from metadata, returns 3 |
| legacy names when `X'4142'` was written, then `'partition.legacy-name' =
'false'` and rows in `X'4344'`, NULL | fails: looks for `p=AB`, the file is
under `p=%5BB@...` | returns 2 of 3 rows, the `X'4142'` row is missing |
| new names when `X'4142'` and NULL were written, then
`'partition.legacy-name' = 'true'` | fails | returns 1 of 2 rows |
| legacy names, rows only in the NULL partition | reads | reads correctly |
So the query succeeds with a partial result and nothing tells the user.
Following the obvious fix, setting `'partition.legacy-name' = 'false'`, does
not make the old partitions readable either; it only moves the loss to the
files written before the change.
**How this PR fixes it.** `PaimonScanPlanProvider.planScanInternal` checks
every data split that will read files (after the splits whose `COUNT(*)` is
answered from metadata) whose binary partition value is not NULL:
- if the split is looked up under legacy names (the options of its table, or
of its branch for a fallback read), planning fails;
- otherwise, if one of its data files was written under a schema whose
options use legacy names (the file's `schemaId`, looked up once per schema and
branch, inside the catalog authentication), planning fails and names that file.
A NULL partition is named after `partition.default-name` under both namings,
and a file with an external path is read from the path its metadata records, so
neither is rejected. For system tables without a schema dictionary (other than
`$ro`; they are read by Paimon's own reader through JNI, which already fails)
only the lookup is checked. The error names the table, the partition and the
option.
**Results.** On the same local cluster, after this PR:
| Query | Doris after this PR |
|---|---|
| `SELECT *` of the legacy table (also with `force_jni_scanner = true`) |
fails: `Cannot read Paimon table d.api_legacy: the table uses legacy partition
names ('partition.legacy-name' = 'true'), under which Paimon names partition p
= 0xEF after a Java array object rather than its bytes, so the files of the
partition are never found. Paimon cannot read the partition either; ...` |
| `SELECT COUNT(*)` of the legacy table | 3, answered from metadata as
before |
| `SELECT * ... WHERE p IS NULL` of the legacy table | the NULL row; Paimon
prunes the other partitions |
| `SELECT *` of the table switched to new names | fails: `... data file
data-b3abc825-...-0.parquet of partition p = 0x4142 was written while the table
used legacy partition names ...`, the file the Paimon API read reports as not
found |
| `SELECT *` of the table switched back to legacy names | fails: `... the
table uses legacy partition names ...` |
| `SELECT *` of the legacy table with only NULL partitions, and of tables
with new names | all rows, the same as the Paimon API read |
`PaimonScanPlanProviderTest` adds three tests: a scan under legacy names
fails while the NULL partition and `COUNT(*)` still plan; a file written under
legacy names fails after the option changes to false, and a file written under
the new names fails after it changes back to true; a file at an external path
still plans and points at an existing file.
`nativeBinaryPartitionsKeepAllPathValues`, added by #68786 for the rendering of
binary partition values, now creates its table with `'partition.legacy-name' =
'false'`: under legacy names the files it planned did not exist. The new tests
fail when the lookup check, the check of the files' schemas, the exemption of
external paths or of NULL values is removed, or when the check moves before the
`COUNT(*)` splits. All 848 tests of fe-connector-paimon pass.
No regression test is added: Doris cannot create a binary column, so the
tables would have to come from the Spark container of the Paimon docker
environment, which was not run locally.
### Release note
Fix: a query of a Paimon table with a binary partition column fails with an
error naming 'partition.legacy-name' when it would read a non-NULL binary
partition whose files Paimon cannot locate under legacy partition names,
instead of silently returning only the rows of the other partitions.
### 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 and written with the Paimon
Catalog API (`Schema.newBuilder().column("p",
DataTypes.BYTES()).partitionKeys("p")`, the option changed with
`SchemaChange.setOption`); the queries above, compared with a read 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 query that reads a non-NULL binary partition of a Paimon table whose
files are named or looked up under legacy partition names fails instead of
returning a partial result.
- 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]
