morningman opened a new pull request, #69295:
URL: https://github.com/apache/doris/pull/69295
### What problem does this PR solve?
Issue Number: None
Related PR: #69083, #69101
Problem Summary:
**In short.** Doris's UPDATE, DELETE and MERGE on a Paimon primary-key table
that stores ORC files miss the rows whose CHAR keys Spark or Flink wrote, when
the key is shorter than the column: the statement commits, and the rows stay,
or a second row with the same key appears. #69083 made Doris pad CHAR values as
Spark and Flink do, but left the rows of ORC tables unpadded, because an UPDATE
or DELETE reads its keys back from ORC without the padding. This PR pads every
row written to an ORC table, UPDATE and DELETE rows included, so that Doris
writes the keys as Spark and Flink do. It also repairs
`test_paimon_write_char_padding`, which fails on master since #69101.
**Background.** ORC stores a CHAR(n) value padded and strips the trailing
spaces when it reads it, so Doris reads back `'k'` for the `'k '` that Spark
wrote. Paimon uses the key bytes as the writer held them in two places: the
bucket of a row is a hash of them, and every data file records its smallest and
largest key, by which reads and compactions decide which files to merge. The
value ORC gives back tells neither.
**The problem, and what it cost.** On a local cluster with a file:// Paimon
catalog, tables with a CHAR(3) primary key whose rows were written through the
Paimon API as Spark writes them (`'k '`, `'ab '`, …), on master:
| Table | `DELETE … WHERE v IN (1, 3, 4, 5)`, then `UPDATE … SET v = v * 10
WHERE v IN (2, 6)` |
|---|---|
| ORC, `bucket = 4` | both COMMITTED, none of the 4 rows deleted |
| ORC, `bucket = -1` (dynamic bucket, one key per bucket) | none deleted,
and `ab` read back twice (2 and 20) |
| ORC, `bucket = 1`: `DELETE … WHERE c = 'ab'`, the smallest key of its file
| not deleted; `UPDATE … WHERE c = 'ab'` leaves two `ab` rows |
| Parquet, all of the above | correct |
Writing the change to the right bucket would not be enough. Through the
Paimon API, a `-D 'k'` written by itself to the bucket that holds `'k '`
deletes nothing: the key range `['k', 'k']` of its file lies below the range
`['k ', 'x ']` of the file holding the row, and Paimon does not merge files
whose ranges do not overlap. A `-D 'x'` in the same bucket, inside that range,
does delete. The key bytes have to be the padded ones. The same happens to keys
with trailing spaces that Doris itself writes, and an `INSERT` of a key Spark
wrote gives a second row with that key.
branch-4.2 writes CHAR values unpadded and buckets UPDATE and DELETE rows
the same way, so it very likely has the same problem; this was not run there.
**How this PR fixes it.**
- `PaimonWriteSchema` (the BE Java writer): on a table that writes ORC
files, by `file.format` or at any level of `file.format.per.level`, every row
is padded, whatever its row kind: INSERT, UPDATE, DELETE and MERGE rows, and
every row of a `rowkind.field` table (whose `rowkind.field` value itself is
still left alone). Other tables keep the rule of #69083: inserted rows are
padded, while UPDATE and DELETE rows keep the bytes Doris read back, which
Parquet and Avro return as stored.
- MERGE: Doris compares a source key with a target key as it reads it. On a
table that writes no ORC files that is the stored, padded key, so #69083
rejects an inserted key shorter than the column. ORC returns the target keys
without their trailing spaces, so there a short key is fine, since MERGE would
have matched a stored `'k '` to `'k'`, and it is padded. What MERGE cannot
insert into an ORC table is a key that ends with a space, such as `'ab '`: it
matched no target key, yet padded it is the stored `'ab '`. Such a MERGE now
fails, and the message points to `rtrim(<key>)` and to
`enable_paimon_write_char_padding`; master silently replaced the stored row.
- FE: `PaimonWriteBinding.charPaddedFields` no longer exempts ORC tables, so
the CHAR bucket key of an ORC table falls back to a single writer, as on other
formats (BE would hash the unpadded value).
- `test_paimon_write_char_padding`: #69101 made `map()` reject STRUCT keys
an hour before #69083 was merged, and the suite has failed at
`t_char_map_row_key` since. Spark now writes those maps into source tables with
STRING fields, and Doris copies them with `INSERT … SELECT`, which still
reaches the writer's map key check. The two cases whose map keys hold a map are
removed: neither Spark nor Doris builds such a map, and `PaimonWriteSchemaTest`
covers them.
**Results.** Rows written by Spark into ORC tables, in the regression suite:
| Statement | master | this PR |
|---|---|---|
| DELETE and UPDATE, `bucket = 4` | 4 rows not deleted | correct |
| DELETE and UPDATE, `bucket = -1` | not deleted, two `ab` rows | correct |
| UPDATE, then DELETE of the smallest key, `bucket = 1` | two `ab` rows,
then one not deleted | correct |
| `INSERT` of keys Spark wrote | two `k` rows and two `ab` rows | one row
each |
| `INSERT … SELECT c, '-D', v` into a `rowkind.field` table | not deleted |
deleted |
| MERGE matching `'k'` | two `k` rows | updated |
| MERGE inserting `'ab '` | replaces the `ab` row Spark wrote | fails,
writes nothing |
Compatibility: Doris up to 4.2 wrote such keys unpadded. On an ORC
primary-key table whose CHAR key is shorter than the column, DML from this
version may miss those rows, as Spark and Flink do. `SET
enable_paimon_write_char_padding = false` writes as before, and `INSERT
OVERWRITE t SELECT * FROM t` rewrites such a table padded. An ORC table
bucketed by a CHAR column is written by one writer, as tables in other formats
are.
### Release note
Doris pads the CHAR values of every row it writes into a Paimon table that
stores ORC files, UPDATE, DELETE and MERGE rows included, as Spark and Flink
do, so that DML reaches the rows they wrote. A MERGE into such a table fails
when it would insert a CHAR primary key ending with a space. Rows that Doris
4.2 or earlier wrote into such a table with a CHAR primary key shorter than the
column may be missed by UPDATE and DELETE; set
`enable_paimon_write_char_padding = false` for those statements, or rewrite the
table with `INSERT OVERWRITE`.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_paimon_write_char_padding` passes on a local cluster against
the iceberg docker environment, and fails with the jars of master at the first
new block (`orc_spark_dml`); the other new blocks were checked by hand with
those jars (table above). The regenerated `.out` differs from master's only by
the new blocks, the two removed cases, and the plan of the ORC table
(`DistributionSpecGather`). The other 31 suites of
`external_table_p0/paimon/write` pass, except
`test_paimon_write_thread_lifecycle` (`/api/be_process_thread_num` returns
nothing on macOS) and `test_paimon_write_variant_errors` (an older local BE);
neither has a CHAR column.
- [x] Unit Test
- `PaimonWriteSchemaTest`, `PaimonWritePlanProviderTest`,
`PaimonWriteBindingTest`; all 146 tests of paimon-scanner and 824 of
fe-connector-paimon pass. The new cases fail on master, and each of these
mutations turns them red: padding only inserted rows or only partition columns
of an ORC table, padding the `rowkind.field`, no MERGE check or the non-ORC
check on an ORC table, checking UPDATE / DELETE rows, routing an ORC CHAR key
natively.
- [ ] 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 -->
- CHAR values written to Paimon ORC tables are padded in every row,
a MERGE inserting a CHAR key that ends with a space into such a table fails,
and such a table bucketed by a CHAR column gets one writer.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
- Together with the documentation of
`enable_paimon_write_char_padding` from #69083.
### 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]
