morningman opened a new pull request, #69083:
URL: https://github.com/apache/doris/pull/69083
### What problem does this PR solve?
Issue Number: None
Related PR: #69045
Problem Summary:
**In short.** Doris wrote CHAR(n) values into Paimon tables without padding
them to n characters, while Spark and Flink pad them, so a primary key that
Doris wrote and the same key written by Spark or Flink were two rows. This PR
pads the CHAR values of inserted rows, nested values and static partition
values included, as Paimon's own cast pads them, behind a new session variable
`enable_paimon_write_char_padding` that is on by default. It came up in the
review of #69045; branch-4.1 and branch-4.2 write CHAR values unpadded too.
**Background.** Paimon's write path stores a value as it is given: it
neither pads nor checks the length of a CHAR value, and it compares primary
keys and hashes buckets by bytes. The engines pad before they write: Spark with
the paimon-spark extensions (`PaimonAnalysis` → `CharVarcharUtils`, `rpad`) and
Flink with an implicit cast (`CharVarCharTrimPadCastRule`), nested CHAR values
included. Paimon's own cast to CHAR (`BinaryStringUtils.toCharacterString`,
used for schema evolution and default values) pads the same way, counting code
points. Doris writes Paimon through the JNI writer (`PaimonJniWriter`, rows
built by `PaimonWriteSchema.tableRow`); the FFI writer backend is a stub that
FE never selects.
**The problem, and what it cost.** On a local cluster with a file:// Paimon
catalog, Parquet tables created through the Paimon API:
| Case | master | with this PR |
|---|---|---|
| Doris inserts `'k'` into a CHAR(3) primary key, then the Paimon API writes
`'k '`, which is what Spark and Flink write for `'k'` | 2 rows: `[k]`, `[k ]`
| 1 row: `[k ]` |
| Doris inserts `['a']`, `map('k', 'v')` and `named_struct('code', 'a', 'n',
1)` into `ARRAY<CHAR(3)>`, `MAP<CHAR(2), CHAR(3)>` and `ROW<code CHAR(3), n
INT>` | `[a]`, `[k] -> [v]`, `[a]` | `[a ]`, `[k ] -> [v ]`, `[a ]` |
| `INSERT OVERWRITE ... PARTITION (p = 'x')` and `INSERT ... VALUES (2,
'y')` into a CHAR(3) partition column | partitions `[x]`, `[y]` | partitions
`[x ]`, `[y ]` |
A table that Doris and Spark or Flink both wrote got duplicate primary keys,
and rows of one key could land in different partitions or buckets.
**How this PR fixes it.**
- **Rows.** `PaimonWriteSchema` pads every CHAR value of an inserted row
with spaces to the column length in code points, as Paimon's cast pads it, and
also the CHAR values nested in ARRAY, MAP and ROW. It builds a padder once per
column whose type holds a CHAR. Unlike Paimon's cast it never cuts a longer
value, because Doris has already decided that: BindSink cuts it when the
session truncates strings on insert and keeps it otherwise. A map keeps every
entry, as before.
- **UPDATE and DELETE.** Their rows are not padded. They carry the key Doris
read back from the table, which has to stay byte-equal to the stored key for
Paimon to match it: padding it would leave a row that an earlier Doris wrote
unpadded neither updated nor deleted. The values an UPDATE sets are written as
before.
- **Static partition values.** `PaimonWriteBinding.resolveStaticPartition`
pads the static partition value of a CHAR column the same way. Paimon parses
that value without padding it (`TypeUtils.castFromString`), so with only the
rows padded the overwrite would name another partition than the rows reach, and
the commit would fail.
- **Switch.** The new session variable `enable_paimon_write_char_padding`
(default true, forwarded with the statement) turns padding off, for a table
whose CHAR keys an earlier Doris wrote unpadded. FE passes it to the BE writer
as the option `doris.write.char-padding` on the table it serializes for the
writer, the table that already carries the write options FE sets, such as
`dynamic-partition-overwrite`; the table the FE committer keeps does not get
the option. So the change needs no thrift field and no BE C++ change. A writer
that finds no such option, from an FE without this change, does not pad, which
matches that FE's unpadded static partition values. BEs are upgraded before
FEs, so a new FE meets new BEs only; a BE without this change would ignore the
option, and a static overwrite of a CHAR partition from a new FE would then
fail its commit rather than write a wrong partition.
Reading is unchanged. Doris reads a padded value with its trailing spaces,
so `WHERE c = 'a'` does not match `'a '`, as with the data that Spark and
Flink write today; with ORC, orc-core strips the trailing spaces on read.
**Results.**
- **FE UT.**
- `PaimonWriteSchemaTest`: top-level CHAR values padded by code points
(`'中'` → `'中 '`), a longer value kept, VARCHAR left alone; CHAR values nested
in ARRAY, MAP and ROW, where a map keeps two keys that only padding makes
equal; padding off; UPDATE and DELETE rows left alone; reading the option.
- `PaimonWriteBindingTest.staticCharPartitionIsPaddedAsTheRowsAre`,
`PaimonBackendBoundTableTest.writeBindingTellsOnlyTheBackendWhetherToPadCharValues`
and
`PaimonWritePlanProviderTest.charPaddingFollowsTheSessionVariableAndDefaultsToOn`.
- Each of these mutations fails its test: no padding, padding every row
kind, no nested padding, ignoring the option, counting bytes or UTF-16 units
instead of code points, not setting the option, and ignoring the session
variable.
- All 737 tests of fe-connector-paimon and 128 of paimon-scanner pass.
- **End to end.** The table above was produced on a local cluster. In the
same run, with the switch off Doris writes `'u'`, `'w'` and the static
partition `'z'` unpadded; with it on again, `DELETE ... WHERE v = 3` removes
`'u'` and `UPDATE ... SET v = 40 WHERE v = 4` updates `'w'` in place, with no
padded duplicate.
- **Regression.** The new `test_paimon_write_char_padding`
(external_table_p0/paimon/write) writes through Spark and Doris: a key both
write is one row, nested and partition values are padded, a second static
overwrite of a CHAR partition replaces the padded partition, the switch off
writes values as they are, and DELETE and UPDATE match the unpadded keys. Its
`.out` was generated on a local cluster with the iceberg docker environment;
with the FE and BE jars from before this change it fails at its first check
(`[k] 1 1` instead of `[k ] 3 1`). The other suites under
`external_table_p0/paimon/write` pass on the same cluster, except
`test_paimon_write_thread_lifecycle`, which reads BE's
`/api/be_process_thread_num` and that endpoint needs Linux `/proc`.
### Release note
Doris now pads CHAR(n) values written into Paimon tables with spaces to n
characters, as Spark and Flink do, including values nested in ARRAY, MAP and
ROW and static partition values; UPDATE and DELETE keep keys as stored. Doris
reads a padded value back with its trailing spaces, so `WHERE c = 'a'` does not
match it, as with data that Spark and Flink write. A CHAR key that an earlier
Doris wrote unpadded is a different key from its padded form; set the session
variable `enable_paimon_write_char_padding` to false to keep writing CHAR
values unpadded.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_paimon_write_char_padding`, as above.
- [x] Unit Test
- `PaimonWriteSchemaTest`, `PaimonWriteBindingTest`,
`PaimonBackendBoundTableTest` and `PaimonWritePlanProviderTest`, as above; all
tests of fe-connector-paimon and paimon-scanner pass.
- [x] Manual test (add detailed scripts or steps below)
- Local single-BE cluster with a file:// Paimon catalog: the cases
in the table above and the switch-off, DELETE and UPDATE cases, before and
after this change.
- [ ] 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 -->
- CHAR values written into Paimon tables are padded to the column
length unless `enable_paimon_write_char_padding` is false.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
- The new session variable `enable_paimon_write_char_padding`.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
--
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]
