Repository navigation
[fix](paimon) Name binary partitions of Paimon writes by their bytes - #69567
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#68874 Problem Summary: Paimon names a partition after the string of each value. With partition.legacy-name = true, the default, that string is toString() of the value, which for a BINARY / VARBINARY value is the Java array object ([B@1b6d3586) rather than its bytes, and every resolution of the name differs. With Paimon 2.1.0, Paimon's own writer and reader cannot round-trip such a partition (the read fails with FileNotFoundException), and a Doris INSERT into such a table had the BE write its files under one [B@... directory, then failed the commit with "Cannot recover from this checkpoint because some files in the snapshot that need to be resubmitted have been deleted", leaving the files behind. A write to a table with a binary partition column and legacy partition names now fails while it is planned, naming the table option 'partition.legacy-name' = 'false' that makes such a table work. With partition.legacy-name = false, Paimon names a binary partition after the text of its bytes, and Doris INSERT already writes and reads it correctly. apache#68874 rejected a static overwrite of a binary partition column with a non-NULL value: Paimon's static overwrite takes the UTF-8 bytes of the value's string, and the cast value reaches the connector as 0x and its hex digits, which names another partition (an overwrite without rows then reported success and left the given partition in place). The connector now decodes the hex digits and passes the text the bytes decode to as UTF-8, which Paimon encodes back to the same bytes. Bytes that are not UTF-8 text, such as 0xEF, are still rejected, since no string names them. ### 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: Unit Test, Manual test - PaimonWriteBindingTest: binary static values become the text of their bytes, bytes that are not UTF-8 are rejected, a write of a legacy-named binary partition fails while one with new names passes - fe-connector-paimon module tests - Manual: local file:// Paimon catalog with tables created by the Paimon API (see the PR description) - Behavior changed: Yes. 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: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed #69567 at 2a790ff. One verified P2 finding: static overwrite of finite BINARY(n)/VARBINARY(n) partitions still fails in the engine's literal cast before the new UTF-8 decoder. The added finite-type tests bypass that cast through a fake handle. The legacy-name admission gate and the strict UTF-8 conversion otherwise match the intended behavior in the inspected paths.
Applicable code-review checkpoints:
- Goal and proof: planning rejects legacy binary partition names before files can be written; unbounded BYTES static values can round-trip through UTF-8. Finite binary types remain incomplete as described inline; unit tests do not prove that production path.
- Scope and reuse: two focused files, one shared binary-type predicate and one conversion helper; connector boundaries are respected.
- Concurrency and locks: no new threads, shared mutable state or locks; conversion state is call-local, binding maps are immutable, and existing transaction synchronization and lock order are unchanged.
- Lifecycle and initialization: validation precedes transaction binding and BE sink creation; failure leaves no writer to close, and existing rollback handles an unbound transaction without payloads. No new native resource or static initialization dependency.
- Configuration: no new Doris setting; the existing Paimon partition.legacy-name option is read from the resolved statement table. Dynamic overwrite and CHAR-padding policies retain their existing behavior.
- Compatibility: no public SPI, function symbol, serialized table format or wire change. HexFormat matches FE's Java 17 target. Rejecting legacy binary writes is the intended behavior change.
- Thrift/protobuf: neither changes; cross-branch field-id checks are inapplicable.
- Parallel paths: INSERT, static/full/dynamic OVERWRITE and row-level UPDATE/DELETE/MERGE share the guarded planWrite/create path. Ordinary INSERT still avoids casting static values.
- Conditions and errors: binary checks apply only to partition fields; SQL NULL bypasses decoding; decoded non-NULL values equal to partition.default-name are rejected; malformed UTF-8 fails before writing. The separate finite-type cast failure is reported inline.
- Test coverage: reviewed all changed tests for BYTES, VARBINARY(2), BINARY(2), malformed/overlong/surrogate UTF-8 and legacy/new naming. Missing real-context finite-length coverage causes the finding. No automated end-to-end binary overwrite test is added.
- Expected results: no regression .out files change; the added SDK conversion assertions are consistent with the inspected SDK code, subject to the engine-cast gap.
- Observability: admission errors identify the table/column and required option; no added per-row logging or new metric is needed for these planning checks.
- Transactions, persistence and failover: no EditLog or durable Doris metadata change; existing Paimon commit, abort and reconciliation boundaries remain in use. No new failover window was identified.
- Writes, atomicity and crashes: legacy rejection occurs before distributed writes; static overwrite retains the existing SDK transaction boundary and compares typed manifest partition values. No additional atomicity or resource-lifetime defect was found in the changed paths.
- FE/BE variables: none added; the existing serialized-table transport and FE static-overwrite map remain the handoff.
- Performance: O(partition fields + static binary bytes) work once per statement, with no added per-row work or remote I/O.
- Core invariants and other risks: native visible-version and MoW bitmap logic are untouched; no new tracked BE allocation is introduced. STRING fallback is unreachable because production Paimon type options force VARBINARY. Empty/blank directory-name aliases do not alias typed overwrite selectors; no second finding was substantiated. This follows the inspected Paimon 2.1.0 partition conversion and typed overwrite predicate.
- User focus and existing threads: no additional user focus or existing inline findings were supplied; live inline-comment readback also found none. There are no existing P0/P1 findings to carry forward.
Review converged after 2 rounds: complete-patch and dedicated binary-risk reviewers rechecked the frozen finding set, with no new valuable findings or unresolved static code questions. Validation was static only: no builds, tests, cluster execution or SDK commit experiments were run. Runtime behavior and the author's reported test/manual results were not independently executed.
Review converged after 2 round(s).
| value = padChar(value, ((CharType) type).getLength()); | ||
| } | ||
| if (isBinary(type)) { | ||
| value = binaryStaticValue(canonicalName, value); |
There was a problem hiding this comment.
[P2] Support finite binary lengths in the static-value cast
For a Paimon table with p BINARY(2) or VARBINARY(2), partitioned by p with partition.legacy-name=false, INSERT OVERWRITE t PARTITION (p = X'4142') SELECT 1 still fails while planning, even though the bytes are valid UTF-8 and fit the column. These source types map to Doris VARBINARY(2), whereas the parser gives the binary literal VarBinaryType.INSTANCE (maximum length). getCastStaticPartitionSpec() calls PluginDrivenInsertCommandContext.writtenValue -> Literal.checkedCastTo; Literal.uncheckedCastTo rejects differing VARBINARY lengths, so the call at line 172 throws before reaching this new conversion. The added finite-type tests supply 0x4142 through a fake handle and bypass that failure. Add byte-preserving binary-to-binary literal casting/retagging and cover the real command context so the newly enabled finite-type overwrites can reach the decoder.
There was a problem hiding this comment.
Confirmed, thanks. Paimon BINARY(n) / VARBINARY(n) map to Doris VARBINARY(n), while the parser types X'4142' as VARBINARY of the maximum length, and castStaticPartitionSpec cast it with Literal.checkedCastTo, which casts a VARBINARY to no other length. The rows were not affected: BindSink's cast of a row value is not folded on FE and BE keeps the bytes. So only the static spec failed, only BYTES reached the decoding this PR adds, and the unit test, which fed the cast value through a fake handle, missed it.
Fixed in 2c95fb189ac: castStaticPartitionSpec gives a binary literal the VARBINARY type of its column with its bytes unchanged, as BE does for the rows. I also checked what Paimon 2.1.0 makes of the value afterwards (its static overwrite parses it with TypeUtils.castFromString): for BINARY(n) it keeps the bytes whatever their length, as the rows keep them; for VARBINARY(n) it fails the commit, after the BE has written its files, when the bytes are longer than n, so the connector now rejects such a value while the write is planned. PluginDrivenTableSinkBindingTest#staticPartitionBinaryValueKeepsItsBytesForAColumnOfAnyLength casts X'41', X'4142' and X'414243' for a VARBINARY(2) column through the sink binding (without the fix it fails with Cast from 4142 to VARBINARY(2) not supported), and PaimonWriteBindingTest#staticBinaryPartitionKeepsTheLengthOfTheBytes covers both length rules. binaryStaticValue also moved so that padChar has its Javadoc again.
TPC-H: Total hot run time: 28432 ms |
TPC-DS: Total hot run time: 150439 ms |
ClickBench: Total hot run time: 24.19 s |
FE Regression Coverage ReportIncrement line coverage |
…ength ### What problem does this PR solve? Issue Number: None Related PR: apache#68874 Problem Summary: Paimon BINARY(n) and VARBINARY(n) columns map to Doris VARBINARY(n), while a binary literal such as X'4142' is VARBINARY of the maximum length. PluginDrivenInsertCommandContext.castStaticPartitionSpec cast a static partition value with Literal.checkedCastTo, which casts a VARBINARY to no other length, so INSERT OVERWRITE ... PARTITION (p = X'4142') on such a column failed while it was planned with "Cast from 4142 to VARBINARY(2) not supported". Only BYTES columns, whose type is VARBINARY of the maximum length, reached the decoding of the previous commit. The rows of the same statement are written with their bytes: BindSink leaves their cast to BE, which keeps them, since a VARBINARY length is metadata only. The cast value of a binary literal now keeps its bytes and takes the column's VARBINARY type, as the rows do. Paimon parses the static value back to those bytes for a BINARY(n) column whatever their length, as the rows carry them; for a VARBINARY(n) column it fails the commit, after the BE wrote its files, when the bytes are longer than n, so such a value is now rejected while the write is planned, naming the column, the value and its length. The Javadoc of padChar, which the previous commit had separated from its method, is moved back. ### Release note None ### Check List (For Author) - Test: Unit Test - PluginDrivenTableSinkBindingTest: X'41', X'4142' and X'414243' keep their bytes when cast for a VARBINARY(2) column through the sink binding - PaimonWriteBindingTest: a BINARY(2) static value keeps one or three bytes, and a VARBINARY(2) value longer than two bytes is rejected - IcebergStaticPartitionLiteralTest, PluginDrivenTableSinkTest, fe-connector-paimon module tests - Behavior changed: Yes. INSERT OVERWRITE ... PARTITION (p = X'...') on a Paimon BINARY(n) / VARBINARY(n) partition column is planned instead of failing on the cast; a value longer than a VARBINARY(n) column fails while planned instead of at commit. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Addressed in
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of all four changed files at 2c95fb1: no new substantiated findings. Existing P2 4366311383 is fixed: the shared command context preserves binary literal bytes when assigning the bound column length, and the new sink-binding test covers that previously failing seam. No existing P0/P1 finding remains to carry forward. No additional user review focus was supplied.
Reviewed flow: PARTITION literal -> bound command context/lazy cast supplier -> Paimon write binding -> BE row bytes and transaction-bound static map -> withOverwrite -> partition predicate. Paimon 2.1.0's binary parser preserves BINARY bytes and checks VARBINARY byte length; its partition predicate selects typed manifest values. The new conversion matches those semantics.
Critical checkpoint conclusions:
- Goal and evidence: source tracing supports planning-time rejection of legacy binary partition names and static overwrites for representable UTF-8 binary values. Added unit cases cover the decoder and real FE sink-binding cast; no runtime proof is claimed.
- Scope and reuse: the changes stay in write admission, binary value conversion, and focused tests. Shared planning remains connector-agnostic.
- Concurrency and locks: new work is statement-local planning with no new thread, shared mutation, lock acquisition, or lock ordering. Existing short synchronized transaction binding is unchanged.
- Lifecycle and initialization: the binding snapshots its maps; decoder/literal temporaries add no resource or static-initialization dependency. Existing JNI/committer ownership remains intact.
- Configuration: no new setting; the guard reads the existing table option partition.legacy-name on the resolved write table.
- Compatibility: no SPI, wire, or storage format changes. Rejecting legacy binary writes is the intended behavior change; existing BE-version admission is preserved.
- Thrift/protobuf field IDs: no gensrc definitions changed; cross-branch field-ID checks are inapplicable.
- Parallel paths: the common binding guard covers all accepted Paimon write operations. Append leaves static cast values alone; full/dynamic and empty-result overwrite retain existing handling. Other connectors consume the unchanged raw spec, so the lazy cast change does not alter their live paths.
- Conditions and errors: binary retagging matches BE's byte-preserving casts. Strict decoding, finite VARBINARY length rejection, and post-decode default-name collision rejection have concrete failure paths. SQL NULL bypasses decoding. Invalid specs fail before BE files are written.
- Test coverage: JUnit cases cover BYTES/BINARY/VARBINARY, finite shorter/equal/longer values, malformed UTF-8, legacy/new naming, and the actual cast supplier. Empty/NUL/supplementary/default-name binary combinations were traced statically; live commit coverage is not added by these tests.
- Expected results: new byte-array and exception assertions agree with the parser/row semantics; no regression .out file changed. Tests were inspected, not executed.
- Observability: admission failures identify the relevant table/column/option or value/length. No new distributed stage or hot-path logging requires additional metrics.
- Transactions, persistence, and failover: validation precedes transaction binding and BE execution. No EditLog or persisted field changed; existing commit, rollback, and reconciliation paths remain intact.
- Data writes and atomicity: the static partition bytes match materialized row bytes, including finite binary lengths. Empty overwrite still commits its static predicate. Existing SDK atomicity and crash handling are unchanged.
- FE/BE variable propagation: no new variable or send point; existing hexadecimal static metadata and raw binary row transport remain consistent.
- Performance and memory: admission scans partition keys once and decoding is linear in static-value bytes, outside per-row execution. No new I/O, retained resource, or significant performance issue was identified.
- Other invariants and module checks: no new optimizer rewrite, slot/nullability rule, visible-version path, MoW/delete-bitmap path, or BE allocation ownership change. No additional issue survived the independent risk and missed-case sweeps.
Validation limits: static inspection only; no build, unit test, regression test, or live-cluster run was performed in this review. The added tests do not supply end-to-end binary overwrite coverage. All four changed files and both initial risk mechanisms were covered; no unresolved static candidate remains.
Review converged after 2 round(s).
TPC-H: Total hot run time: 28495 ms |
TPC-DS: Total hot run time: 150765 ms |
ClickBench: Total hot run time: 24.19 s |
FE Regression Coverage ReportIncrement line coverage |
…ames (#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) and is also missing at its planned path (`bucketPath/fileName`, the path the native reader is given), planning fails and names that file. The schema alone does not prove how a file was named: a writer's dynamic options (`spark.paimon.*`, Flink hints, `Table.copy`) keep the schema ID, so a file written with a dynamic `'partition.legacy-name' = 'false'` on a table that keeps the default refers to a schema with legacy names while it sits under the bytes of its value, and it is read once the table uses those names; - and, on a table with `'index-file-in-data-file-dir' = 'true'`, every deletion vector the split carries must exist, whether or not its data file has an external path, or planning fails and names the deletion vector file and its data file. Such a table keeps its deletion vectors with its data files: one written while no external data path is configured records only its file name and is looked up under the directory of its partition, which a legacy name never finds, while its data file keeps its external path, and the BE skips the whole data file when it cannot open the deletion vector. The option is immutable, so every writer used the reading table's value. Deletion vectors under the table's `index/` directory do not depend on partition names and are not checked. A NULL partition is named after `partition.default-name` under both namings, and a data file with an external path is read from the path its metadata records, so neither is rejected; the deletion vectors of a file with an external path are still checked. The system tables read through JNI (`$audit_log`, `$row_tracking`) report the options of the main branch, while a fallback split is planned and read under its own branch, so the check resolves the branch that produced each split from the pinned source of the wrapper, as it does for data tables. `$binlog` exposes its partition columns as arrays and is not checked; Paimon's own reader fails on a file it cannot find. A `scan.file-creation-time-millis` read of a fallback table plans the main branch through its `SnapshotReader`, and its plain splits are judged under the main branch. The error names the table, the partition and the option. The existence checks run inside the catalog authentication, once per path and scan: a table that keeps its index files under `index/` and whose files were written under its current naming makes no extra filesystem calls. **Not covered.** A file written with a dynamic `'partition.legacy-name' = 'true'` (or by a Paimon writer too old to know the option) while its schema says `'false'` sits under a legacy name but is looked up under its bytes, and the BE still skips it, as on master and branch-4.2. Catching it would take an existence call for every file of every non-NULL binary partition on every scan. **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 eight 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; a file at an external path whose deletion vector was written after the external paths were removed (persisted the way `PaimonSparkWriter.persistDeletionVectors` does it for a DELETE) fails and names the deletion vector, while a deletion vector written with the external paths in place still plans and points at an existing file; a file written through `table.copy({"partition.legacy-name": "false"})` on a table that keeps the default fails as a legacy lookup, then plans once the option is altered to `false`; a fallback table whose main branch holds only a NULL partition and whose fallback branch holds a non-NULL binary partition plans the data table and `$audit_log` when the fallback branch names partitions by bytes, and the planned `$audit_log` splits read both rows back through Paimon, while both fail at planning with the legacy-name error when only the main branch names them by bytes; and a `scan.file-creation-time-millis` read of a fallback table with a non-NULL binary partition plans the main branch's 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, when the check moves before the `COUNT(*)` splits, when the deletion vectors are not checked, when every deletion vector fails without its existence being checked, when a file whose schema uses legacy names fails without its existence being checked, when a system table read through JNI is checked under the wrapper's options, when every split is judged under the main branch, or when a split that does not come from the fallback scan is cast to a fallback split (the file-creation-time read then failed with a `ClassCastException`). The 853 tests of fe-connector-paimon run without failures (one is skipped, as before). 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.
### What problem does this PR solve? Issue Number: None Related PR: #69567 Problem Summary: **In short.** A CASE WHEN whose result type is VARBINARY and that has two or more WHEN branches failed in BE with `argument_type VarBinary not supported`. BE now selects VARBINARY values the way it selects strings. **Background.** VARBINARY values reach queries through external tables (Iceberg, Paimon and Hive binary columns map to VARBINARY) and through `CAST(... AS VARBINARY)`. BE evaluates CASE WHEN in `VCaseExpr`: it computes, for every row, the index of the branch that the row takes, and then `VCaseExpr::_execute_update_result` builds the result column, dispatching on the result's primitive type to a typed copy loop. Nereids rewrites a CASE with a single WHEN and an ELSE into `if()`, whose generic path copies values through `IColumn::insert_from` and accepts any column type. **The problem, and what it cost.** The dispatch in `_execute_update_result` had no entry for `TYPE_VARBINARY`, so it fell through to `NOT_IMPLEMENTED_ERROR`. Every query with such a CASE failed, whatever the constant-folding mode (the CASE is evaluated by BE, not folded), for example ```sql SELECT CASE WHEN number % 3 = 0 THEN CAST(concat('a', number) AS VARBINARY) WHEN number % 3 = 1 THEN CAST('y' AS VARBINARY) ELSE X'FF' END FROM numbers("number" = "6"); -- ERROR 1105 (HY000): errCode = 2, detailMessage = (127.0.0.1)[NOT_IMPLEMENTED_ERROR][E3] -- argument_type Nullable(VarBinary) not supported ``` Only a CASE with one WHEN and an ELSE worked, because it is evaluated as `if()`. branch-4.2 has the same gap. **How this PR fixes it.** `TYPE_VARBINARY` now maps to `ColumnVarbinary`, and `ColumnVarbinary` takes the per-row copy (`update_result_normal`) that CASE already uses for strings, both for a nullable result (including CASE without ELSE, whose unmatched rows are NULL) and for a non-nullable one. FE needs no change. **Results.** | CASE WHEN with a VARBINARY result | Before | After | |---|---|---| | Two or more WHEN branches, with ELSE | fails: `argument_type VarBinary not supported` | returns the selected bytes | | Two or more WHEN branches, without ELSE | fails: `argument_type Nullable(VarBinary) not supported` | returns the selected bytes, NULL for unmatched rows | | One WHEN with ELSE (rewritten to `if()`) | works | works (unchanged) | - BE unit tests: `VCaseVarbinaryTest` adds two cases through `_execute_update_result`: a non-nullable result with ELSE (a column branch, a constant branch, NUL and non-UTF-8 bytes) and a nullable result without ELSE (NULL from a branch and from unmatched rows). Both fail without the change with the error above. `VCaseSelectionTest` still passes (50 cases in total). - Regression test: `query_p0/sql_functions/binary_functions/test_varbinary_case_when` runs CASE with and without ELSE over a table, over `numbers()` and on constants, in the three constant-folding modes (FE folding, folding by BE, no folding); the results match in all modes.
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 istoString()of the value, which for a binary value is the Java array object ([B@1b6d3586) rather than its bytes. Withpartition.legacy-name = falseit is the text of the bytes (p=ABforX'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:legacy-name = true, writeX'4142', then readFileNotFoundException(every resolution of the name gives another[B@...)INSERTfails 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 underp=%5BB@...legacy-name = false,INSERTofX'4142',X'EF', NULL (alsoBINARY(4), and a multi-instanceINSERT ... SELECT)legacy-name = false,INSERT OVERWRITE ... PARTITION (p = X'4142') SELECT ...Static overwrite of binary partition column 'p' is not supportedThe rejection came from #68874: the cast value of a binary static partition reaches the connector as
0xand 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.resolveStaticPartitiondecodes 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 lone0xEF, an overlong or surrogate sequence), which no string names, are still rejected, with the value in the message. A NULL static value keeps naming thepartition.default-namepartition, and a text equal to that name is still rejected, as for other types.PluginDrivenInsertCommandContext.castStaticPartitionSpec(fe-core) gives a binary literal the VARBINARY type of its column with its bytes unchanged. A literal is VARBINARY of the maximum length, whichLiteral.checkedCastTocasts to no other length, so the static value of a Paimon BINARY(n) / VARBINARY(n) column (VARBINARY(n) in Doris) failed on the cast while the rows, whose cast BindSink leaves to BE, keep their bytes. Paimon's static overwrite parses the value back to the same bytes for a BINARY(n) column whatever their length, and fails the commit for a VARBINARY(n) value longer than n, so the connector rejects such a value while the write is planned. The cast serves the static partitions of every plugin connector; the only other binary partition type it reaches is an Iceberg FIXED(n) column (VARBINARY(n) in Doris), whose static value failed on the same cast and now reaches the Iceberg connector, which rejects a value of another length when its commit parses the partition values (Invalid fixed partition length).Results. On the same local cluster, with a
legacy-name = falsetable holding(1, X'4142'),(2, X'4344'),(3, X'EF'),(4, NULL),(6, X''):INSERT OVERWRITE ... PARTITION (p = X'4142') SELECT 5X'4142'partition holds only 5INSERT OVERWRITE ... PARTITION (p = X'4344') SELECT 1 LIMIT 0X'4344'partition is empty, the others are untouchedINSERT OVERWRITE ... PARTITION (p = X'EF') SELECT 1Static partition value 0xEF of binary column 'p' is not UTF-8 text, ...; 3 staysINSERT OVERWRITE ... PARTITION (p = NULL) SELECT 9INSERT OVERWRITE ... PARTITION (p = X'') SELECT 7INSERT/INSERT OVERWRITEinto alegacy-name = truetable... its binary partition column 'p' needs the table option 'partition.legacy-name' = 'false' ...; no files are writtenPaimonWriteBindingTestreplaces the test of the #68874 rejection with four tests: a binary static value becomes the text of its bytes and Paimon parses it back to them (forBYTES,VARBINARY(2),BINARY(2)); bytes that are not UTF-8 are rejected; a BINARY(2) value keeps one or three bytes while a VARBINARY(2) value longer than two bytes is 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.PluginDrivenTableSinkBindingTestcastsX'41',X'4142'andX'414243'for a VARBINARY(2) column through the sink binding. They fail when the legacy check is dropped, when the cast value is passed through, when the decoding replaces malformed bytes, when the length check is dropped, and when fe-core casts the literal withcheckedCastTo. All 848 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 (BINARY, VARBINARY or BYTES) is supported when the bytes are UTF-8 text and, for VARBINARY(n), at most n bytes long.Check List (For Author)
Test
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 withoutpartition.legacy-name = false); the statements in the tables above, read back through Doris and through the Paimon API. The BINARY(n) / VARBINARY(n) static overwrite of the follow-up commit is covered by unit tests only; it was not rerun on the cluster.Behavior changed:
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, for BINARY(n) and VARBINARY(n) columns as well as BYTES; a value longer than a VARBINARY(n) column fails at planning instead of at commit.
Does this need documentation?
Check List (For Reviewer who merge this PR)