Skip to content

[fix](paimon) Name binary partitions of Paimon writes by their bytes - #69567

Merged
morningman merged 2 commits into
apache:masterfrom
morningman:b2m-t14f-paimon-binary-partition
Oct 25, 2026
Merged

morningman merged 2 commits into
apache:masterfrom
morningman:b2m-t14f-paimon-binary-partition

Conversation

@morningman

@morningman morningman commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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.
  • 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, which Literal.checkedCastTo casts 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 = 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 four 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; 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. PluginDrivenTableSinkBindingTest casts X'41', X'4142' and X'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 with checkedCastTo. 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

    • Regression test
    • Unit Test
    • 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. 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.
    • 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
  • Behavior changed:

    • No.
    • 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, 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?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### 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]>
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28432 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2a790ff07a2257e1cba6117eb4e08b7d942c78f2, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17600	5115	5008	5008
q2	2095	343	241	241
q3	10230	1404	824	824
q4	4685	456	340	340
q5	7436	791	535	535
q6	185	181	156	156
q7	744	790	607	607
q8	9245	1287	1100	1100
q9	5443	4157	4305	4157
q10	6828	1287	1035	1035
q11	434	250	224	224
q12	640	401	279	279
q13	18080	3614	3013	3013
q14	271	277	255	255
q15	q16	736	708	687	687
q17	1222	744	575	575
q18	7616	6178	6147	6147
q19	1111	964	617	617
q20	394	347	238	238
q21	4921	2148	2298	2148
q22	348	291	246	246
Total cold run time: 100264 ms
Total hot run time: 28432 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5567	5570	5568	5568
q2	656	576	522	522
q3	5202	5354	4973	4973
q4	1974	2118	1552	1552
q5	5267	5181	5612	5181
q6	273	214	151	151
q7	2372	2079	1867	1867
q8	3049	2846	2708	2708
q9	8043	8000	8003	8000
q10	3720	3655	3236	3236
q11	619	431	423	423
q12	688	686	513	513
q13	3375	3902	3250	3250
q14	299	298	284	284
q15	q16	720	716	639	639
q17	8328	7548	7323	7323
q18	12988	12197	12938	12197
q19	901	775	758	758
q20	2124	2121	1889	1889
q21	5569	4636	4685	4636
q22	496	437	399	399
Total cold run time: 72230 ms
Total hot run time: 66069 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150439 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 2a790ff07a2257e1cba6117eb4e08b7d942c78f2, data reload: false

query5	4317	573	439	439
query6	427	213	192	192
query7	4798	450	227	227
query8	323	174	152	152
query9	8775	3075	3089	3075
query10	470	306	262	262
query11	5941	3877	3552	3552
query12	149	94	88	88
query13	1249	451	335	335
query14	6126	3939	3643	3643
query14_1	3402	3404	3382	3382
query15	210	209	181	181
query16	943	480	430	430
query17	874	670	560	560
query18	2420	430	313	313
query19	196	175	138	138
query20	88	87	84	84
query21	208	127	109	109
query22	12957	13007	12756	12756
query23	13510	13037	12470	12470
query23_1	12672	12566	12641	12566
query24	7345	824	465	465
query24_1	470	496	481	481
query25	547	428	365	365
query26	1248	256	140	140
query27	2770	447	274	274
query28	4613	1937	1894	1894
query29	1578	621	450	450
query30	296	224	181	181
query31	991	931	824	824
query32	139	93	90	90
query33	534	300	250	250
query34	918	842	495	495
query35	748	760	645	645
query36	825	806	706	706
query37	121	100	84	84
query38	1772	1727	1642	1642
query39	794	791	758	758
query39_1	757	734	751	734
query40	221	118	99	99
query41	71	69	68	68
query42	87	90	85	85
query43	347	362	307	307
query44	1314	684	687	684
query45	178	180	167	167
query46	815	973	603	603
query47	2937	2962	2689	2689
query48	285	307	211	211
query49	570	399	287	287
query50	662	269	195	195
query51	10313	10686	10430	10430
query52	83	80	69	69
query53	179	212	149	149
query54	227	206	182	182
query55	70	64	61	61
query56	224	207	219	207
query57	1636	1643	1568	1568
query58	286	262	255	255
query59	2126	2174	1920	1920
query60	278	226	220	220
query61	148	144	141	141
query62	383	326	285	285
query63	186	157	159	157
query64	2724	920	734	734
query65	3298	3265	3254	3254
query66	1764	432	299	299
query67	20111	19977	20116	19977
query68	3021	980	598	598
query69	381	280	234	234
query70	838	756	770	756
query71	298	232	209	209
query72	2808	2641	2303	2303
query73	502	522	291	291
query74	4645	4517	4341	4341
query75	2342	2271	1911	1911
query76	2320	1016	584	584
query77	339	369	265	265
query78	9676	9695	9206	9206
query79	960	894	523	523
query80	782	440	349	349
query81	574	311	275	275
query82	587	130	112	112
query83	271	191	181	181
query84	305	121	100	100
query85	964	429	378	378
query86	352	235	232	232
query87	2009	1939	1815	1815
query88	3633	2639	2608	2608
query89	329	282	243	243
query90	1704	167	163	163
query91	163	150	123	123
query92	93	87	80	80
query93	1010	993	572	572
query94	502	287	253	253
query95	568	389	299	299
query96	659	477	232	232
query97	2452	2462	2315	2315
query98	162	151	143	143
query99	773	764	665	665
Total cold run time: 233680 ms
Total hot run time: 150439 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.19 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2a790ff07a2257e1cba6117eb4e08b7d942c78f2, data reload: false

query1	0.00	0.00	0.00
query2	0.09	0.04	0.04
query3	0.26	0.10	0.08
query4	1.62	0.10	0.10
query5	0.22	0.24	0.24
query6	1.17	0.69	0.68
query7	0.04	0.01	0.00
query8	0.05	0.04	0.04
query9	0.40	0.33	0.34
query10	0.56	0.62	0.58
query11	0.19	0.14	0.15
query12	0.18	0.15	0.14
query13	0.49	0.48	0.48
query14	1.03	1.00	1.01
query15	0.64	0.61	0.61
query16	0.34	0.36	0.33
query17	1.18	1.12	1.10
query18	0.19	0.20	0.19
query19	2.12	2.01	1.99
query20	0.02	0.01	0.02
query21	15.43	0.20	0.13
query22	4.87	0.05	0.05
query23	16.28	0.32	0.12
query24	2.91	0.56	0.37
query25	0.10	0.04	0.06
query26	1.22	0.22	0.16
query27	0.06	0.03	0.03
query28	3.61	0.79	0.36
query29	12.47	4.19	3.28
query30	0.28	0.13	0.10
query31	2.77	0.54	0.34
query32	3.21	0.60	0.51
query33	3.12	3.23	3.17
query34	15.49	3.99	3.37
query35	3.34	3.32	3.33
query36	0.56	0.44	0.41
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.15	0.13
query41	0.09	0.03	0.03
query42	0.04	0.02	0.03
query43	0.04	0.04	0.04
Total cold run time: 97.03 s
Total hot run time: 24.19 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/14) 🎉
Increment coverage report
Complete coverage report

…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]>
@morningman

Copy link
Copy Markdown
Contributor Author

Addressed in 2c95fb189ac:

  • 4366311383 (binary static values of a finite-length column): fe-core now casts a binary static partition value to the VARBINARY type of its column with its bytes unchanged, so INSERT OVERWRITE ... PARTITION (p = X'4142') plans on Paimon BINARY(n) / VARBINARY(n) columns, not only on BYTES. A value longer than a VARBINARY(n) column, which Paimon's static overwrite rejects at commit, is rejected while the write is planned. This PR now also changes fe-core (PluginDrivenInsertCommandContext).

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.
  2. Scope and reuse: the changes stay in write admission, binary value conversion, and focused tests. Shared planning remains connector-agnostic.
  3. 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.
  4. Lifecycle and initialization: the binding snapshots its maps; decoder/literal temporaries add no resource or static-initialization dependency. Existing JNI/committer ownership remains intact.
  5. Configuration: no new setting; the guard reads the existing table option partition.legacy-name on the resolved write table.
  6. Compatibility: no SPI, wire, or storage format changes. Rejecting legacy binary writes is the intended behavior change; existing BE-version admission is preserved.
  7. Thrift/protobuf field IDs: no gensrc definitions changed; cross-branch field-ID checks are inapplicable.
  8. 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.
  9. 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.
  10. 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.
  11. Expected results: new byte-array and exception assertions agree with the parser/row semantics; no regression .out file changed. Tests were inspected, not executed.
  12. Observability: admission failures identify the relevant table/column/option or value/length. No new distributed stage or hot-path logging requires additional metrics.
  13. 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.
  14. 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.
  15. FE/BE variable propagation: no new variable or send point; existing hexadecimal static metadata and raw binary row transport remain consistent.
  16. 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.
  17. 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).

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28495 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2c95fb189ac2ab41384a45126d0399d68ff8a3a6, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17583	5157	5053	5053
q2	2180	314	263	263
q3	10229	1373	833	833
q4	4672	465	333	333
q5	7433	797	537	537
q6	183	187	149	149
q7	735	799	601	601
q8	9266	1320	1095	1095
q9	5584	4236	4194	4194
q10	6843	1272	1036	1036
q11	419	248	227	227
q12	648	397	291	291
q13	18065	3594	3006	3006
q14	276	273	252	252
q15	q16	746	730	662	662
q17	1189	746	589	589
q18	6932	6151	6109	6109
q19	1092	943	583	583
q20	399	338	235	235
q21	5549	2191	2294	2191
q22	346	292	256	256
Total cold run time: 100369 ms
Total hot run time: 28495 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5619	5576	5624	5576
q2	682	564	554	554
q3	5204	5428	4906	4906
q4	1998	2150	1539	1539
q5	5292	5182	5626	5182
q6	266	197	152	152
q7	2200	2071	1890	1890
q8	3069	2714	2658	2658
q9	7963	7958	7952	7952
q10	3743	3658	3221	3221
q11	601	508	435	435
q12	681	693	548	548
q13	3554	3955	3247	3247
q14	338	331	300	300
q15	q16	710	727	659	659
q17	8197	7380	6892	6892
q18	12999	12155	12821	12155
q19	919	803	820	803
q20	2134	2150	1940	1940
q21	5768	4591	4686	4591
q22	497	459	419	419
Total cold run time: 72434 ms
Total hot run time: 65619 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150765 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 2c95fb189ac2ab41384a45126d0399d68ff8a3a6, data reload: false

query5	4316	613	431	431
query6	444	210	201	201
query7	4829	437	220	220
query8	313	165	148	148
query9	8777	3141	3092	3092
query10	490	304	256	256
query11	5955	3849	3559	3559
query12	147	88	85	85
query13	1290	445	326	326
query14	6097	3951	3654	3654
query14_1	3432	3443	3374	3374
query15	210	192	180	180
query16	961	437	397	397
query17	837	632	532	532
query18	2412	427	294	294
query19	193	154	132	132
query20	82	82	86	82
query21	204	129	112	112
query22	12968	13085	12842	12842
query23	13343	12730	12465	12465
query23_1	12610	12433	12469	12433
query24	7379	818	458	458
query24_1	486	480	470	470
query25	545	457	357	357
query26	1253	261	138	138
query27	2765	442	273	273
query28	5028	1984	2016	1984
query29	1583	628	466	466
query30	312	226	189	189
query31	1005	930	813	813
query32	159	96	97	96
query33	531	307	245	245
query34	894	887	515	515
query35	741	773	642	642
query36	818	814	731	731
query37	128	97	88	88
query38	1787	1741	1689	1689
query39	793	792	740	740
query39_1	729	730	739	730
query40	219	117	96	96
query41	70	68	69	68
query42	90	82	83	82
query43	346	369	308	308
query44	1310	685	689	685
query45	178	180	166	166
query46	832	960	554	554
query47	2690	2743	2798	2743
query48	302	313	222	222
query49	587	420	298	298
query50	695	289	223	223
query51	10582	10537	10477	10477
query52	88	89	78	78
query53	188	210	155	155
query54	258	200	192	192
query55	72	69	66	66
query56	246	226	228	226
query57	1651	1618	1574	1574
query58	297	271	256	256
query59	2107	2202	1978	1978
query60	307	255	244	244
query61	171	172	165	165
query62	387	340	302	302
query63	189	163	164	163
query64	2892	951	777	777
query65	3334	3283	3264	3264
query66	1772	428	303	303
query67	20285	19876	20077	19876
query68	2973	1000	607	607
query69	390	303	240	240
query70	843	804	781	781
query71	295	228	195	195
query72	2741	2544	2290	2290
query73	528	529	297	297
query74	4652	4521	4345	4345
query75	2331	2321	1896	1896
query76	2327	1001	590	590
query77	346	379	266	266
query78	9671	9789	9184	9184
query79	998	843	522	522
query80	731	424	353	353
query81	543	322	264	264
query82	584	138	119	119
query83	294	193	181	181
query84	322	117	100	100
query85	902	420	364	364
query86	357	250	227	227
query87	2007	1960	1810	1810
query88	3668	2679	2636	2636
query89	327	280	238	238
query90	1801	170	169	169
query91	166	147	133	133
query92	100	80	90	80
query93	995	991	565	565
query94	497	306	272	272
query95	569	383	297	297
query96	674	484	229	229
query97	2411	2432	2314	2314
query98	160	149	148	148
query99	777	758	662	662
Total cold run time: 234741 ms
Total hot run time: 150765 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.19 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2c95fb189ac2ab41384a45126d0399d68ff8a3a6, data reload: false

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.25	0.09	0.08
query4	1.60	0.09	0.10
query5	0.23	0.24	0.24
query6	1.16	0.70	0.66
query7	0.03	0.01	0.00
query8	0.06	0.04	0.04
query9	0.40	0.34	0.34
query10	0.59	0.57	0.60
query11	0.20	0.14	0.16
query12	0.18	0.14	0.15
query13	0.49	0.49	0.49
query14	1.02	1.04	1.01
query15	0.63	0.61	0.61
query16	0.32	0.33	0.36
query17	1.13	1.15	1.12
query18	0.21	0.18	0.19
query19	2.07	2.01	1.93
query20	0.02	0.02	0.01
query21	15.43	0.21	0.13
query22	4.91	0.06	0.05
query23	16.30	0.33	0.12
query24	2.84	0.44	0.40
query25	0.09	0.07	0.06
query26	1.04	0.21	0.16
query27	0.05	0.03	0.03
query28	3.92	0.83	0.37
query29	12.48	4.17	3.24
query30	0.27	0.12	0.11
query31	2.77	0.55	0.33
query32	3.22	0.60	0.51
query33	3.17	3.16	3.17
query34	15.54	3.99	3.39
query35	3.35	3.34	3.32
query36	0.57	0.44	0.44
query37	0.09	0.06	0.06
query38	0.04	0.04	0.03
query39	0.04	0.02	0.02
query40	0.18	0.14	0.14
query41	0.08	0.03	0.03
query42	0.03	0.03	0.04
query43	0.04	0.03	0.03
Total cold run time: 97.14 s
Total hot run time: 24.19 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/2) 🎉
Increment coverage report
Complete coverage report

@morningman
morningman merged commit 0a2f7fd into apache:master Oct 25, 2026
44 checks passed
morningman added a commit that referenced this pull request Oct 26, 2026
…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.
morningman added a commit that referenced this pull request Oct 26, 2026
### 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants