[refactor](storage) move fixed partial-update fill into FixedPartialUpdateFillStage by csun5285 · Pull Request #66609 · apache/doris

csun5285 · 2026-08-10T07:54:41Z

Fourth PR of the #64674 split stack (#65492, #66151, #66528). The fixed partial-update fill moves out of both segment writers into the transform chain: fixed PU loads now run [Validate, FixedPartialUpdateFill, VariantParse, RowStoreFill], so the fill widens the narrow block, probes each key via MowKeyProbe::for_partial_update, reads old rows through a per-block HistoricalRowFetcher, and hands the writers a plain full-width upsert.

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

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

hello-stephen · 2026-08-10T07:54:47Z

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:

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

csun5285 · 2026-08-10T07:57:11Z

run buildall

csun5285 · 2026-08-10T07:57:17Z

/review

csun5285 · 2026-08-10T08:01:32Z

/review

github-actions

Request changes: one fixed-partial-update hot-path regression remains.

Critical checkpoint conclusions:

Goal and test proof: The logical fill migration is functionally coherent. New and existing coverage exercises history/default/sequence/delete behavior, exact marks and stats, vertical persistence and PK bytes, multi-segment missing-sequence persistence, and combined sequence/Variant/RowStore persistence. Per the review bundle, I did not run local builds or tests. Current CI has formatting, CheckStyle, license, dependency, and secret checks green; BE UT (Linux/macOS), compile, and performance are still pending.
Scope: The production change is confined to the fixed-partial-update transform/writer seam and related tests; no unrelated behavior change was found.
Concurrency and lifecycle: Transform/fetch state is flush-local, shared totals are atomic, delete-bitmap mutations are locked, RowStore generation owns its COW snapshot, and failures after transform remain terminal before rowset commit.
Config and compatibility: No config, FE/BE variable, protocol, or storage-format contract changes were introduced; no rolling or persisted-data compatibility concern was found.
Parallel paths and special conditions: Horizontal and vertical writers, flexible updates, row-binlog groups, cloud/transient paths, streaming rejection, compaction, and schema change were traced. Fixed fill runs exactly once on one fresh segment, while specialized paths retain their intended ownership.
Observability and write correctness: Counters, correctness sentinels, TEMP delete marks, transaction visibility, historical/default fill, supplied and missing sequences, deletes, new-key policy, null/default/auto-increment values, final PK construction, Variant parsing, and RowStore serialization remain coherent.
Performance: The inline P2 is blocking. Every fixed update now repeats primary-key conversion in the generic writer; no-sequence and supplied-sequence cases also repeat final-key encoding and conditional row-cache erasure.
Other issues and focus: No additional user focus was supplied, so the whole PR was reviewed. No other distinct issue survived duplicate suppression and two converged review rounds.

github-actions

Comment-only duplicate-suppressed closeout for this review run.

The current head already has CHANGES_REQUESTED review 4894972388 with inline comment 3747875632 covering the one substantiated P2: fixed partial updates now repeat key conversion in both generic writers, and no-sequence/supplied-sequence cases also repeat final-key encoding plus conditional RowCache erasure. This run independently reached the same finding, then suppressed it under the existing-thread rule. No new distinct inline issue remains after two converged review rounds.

Critical checkpoint conclusions:

Goal and test proof: The fixed-PU fill migration is functionally coherent. The added tests and unchanged segment-format coverage exercise historical/default fill, sequence winner/loser and omitted/supplied suffixes, delete signs, exact TEMP marks and stats, horizontal/vertical persistence, Variant parsing, and RowStore serialization. Per the review bundle, no local builds or tests were run.
Scope: The production change is focused on the fixed-PU transform/writer seam and related tests; no unrelated behavior change was found.
Concurrency and lifecycle: Each successful fixed fill owns one fresh segment at row position zero; concurrent flushes use distinct atomic segment ids and flush-local transform/fetch state. Rowset pins, the RowStore COW snapshot, atomic counters, sticky failure handling, and abort cleanup have consistent lifetimes.
Configuration and compatibility: No new configuration, FE/BE variable, protocol, storage-format, or rolling-upgrade contract is introduced. Horizontal/vertical selection remains dynamic and both paths were reviewed.
Parallel and special paths: Flexible partial update, row-binlog/group writers, transient/publish, streaming rejection, compaction, schema change, empty blocks, multi-segment coordinates, and downstream failure paths were traced; no distinct regression survived review.
Write correctness and observability: MOW lookup outcomes, delete-bitmap marks and sentinels, new-key policy, required/default/nullable/auto-increment fill, old-deleted sequence preservation, final PK construction, row/meta counts, and existing stats/logging remain coherent.
Performance: The already-landed P2 remains the only blocking concern. It was not duplicated here.
User focus: No additional review focus was supplied, so the complete PR was reviewed.

Review status: complete and converged; zero new inline comments after live duplicate suppression.

hello-stephen · 2026-08-10T10:37:58Z

TPC-H: Total hot run time: 28952 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 8bce8194b7a3a80c4071d7e615c18465fef2355d, data reload: false
------ Round 1 ----------------------------------
============================================
q1	17682	3937	3896	3896
q2	2096	324	195	195
q3	10277	1382	806	806
q4	4682	468	335	335
q5	7526	859	556	556
q6	179	168	133	133
q7	758	800	618	618
q8	9334	1721	1607	1607
q9	5372	4099	4079	4079
q10	6770	1630	1385	1385
q11	518	349	332	332
q12	713	574	447	447
q13	18109	3286	2757	2757
q14	267	264	249	249
q15	q16	737	730	663	663
q17	998	1008	978	978
q18	6676	5609	5563	5563
q19	1346	1170	1049	1049
q20	784	665	577	577
q21	5846	2525	2429	2429
q22	430	359	298	298
Total cold run time: 101100 ms
Total hot run time: 28952 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4277	4207	4193	4193
q2	276	318	210	210
q3	4627	5329	4363	4363
q4	2157	2234	1407	1407
q5	4284	4089	4112	4089
q6	228	172	126	126
q7	1700	1570	1405	1405
q8	2979	2180	2102	2102
q9	7512	7481	7513	7481
q10	4320	4233	3895	3895
q11	554	400	369	369
q12	719	738	523	523
q13	3131	3511	2880	2880
q14	299	306	267	267
q15	q16	678	716	617	617
q17	1303	1271	1287	1271
q18	12087	11057	11833	11057
q19	1191	1249	1151	1151
q20	2226	2206	1929	1929
q21	5665	4818	4797	4797
q22	545	487	434	434
Total cold run time: 60758 ms
Total hot run time: 54566 ms

hello-stephen · 2026-08-10T10:48:15Z

TPC-DS: Total hot run time: 158519 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 8bce8194b7a3a80c4071d7e615c18465fef2355d, data reload: false
query5	4321	587	450	450
query6	455	225	213	213
query7	4886	634	342	342
query8	320	162	153	153
query9	8791	4020	4021	4020
query10	451	357	309	309
query11	5804	2179	1983	1983
query12	153	102	96	96
query13	1296	605	439	439
query14	6067	4279	4006	4006
query14_1	3839	3865	3860	3860
query15	211	192	172	172
query16	1008	525	463	463
query17	918	720	570	570
query18	2447	473	353	353
query19	212	206	151	151
query20	104	104	112	104
query21	232	159	141	141
query22	12954	13061	12817	12817
query23	15764	14964	14553	14553
query23_1	14650	14720	14625	14625
query24	7550	1724	1252	1252
query24_1	1288	1250	1243	1243
query25	556	453	382	382
query26	1327	364	217	217
query27	2603	597	369	369
query28	4592	2073	2061	2061
query29	1082	626	498	498
query30	343	266	227	227
query31	1176	1121	1090	1090
query32	116	71	61	61
query33	525	318	259	259
query34	1179	1096	638	638
query35	724	748	639	639
query36	811	768	715	715
query37	162	108	97	97
query38	1827	1756	1679	1679
query39	816	813	804	804
query39_1	795	799	796	796
query40	252	174	151	151
query41	74	71	70	70
query42	99	96	96	96
query43	322	324	284	284
query44	1444	768	779	768
query45	188	220	170	170
query46	1090	1219	709	709
query47	1551	1492	1453	1453
query48	405	425	285	285
query49	585	412	287	287
query50	1161	416	321	321
query51	10474	10529	10486	10486
query52	87	85	75	75
query53	260	270	196	196
query54	288	231	228	228
query55	73	71	67	67
query56	315	289	277	277
query57	1026	991	945	945
query58	274	271	259	259
query59	1546	1605	1405	1405
query60	300	281	253	253
query61	154	153	153	153
query62	394	321	261	261
query63	239	202	193	193
query64	2870	1022	851	851
query65	3896	3839	3797	3797
query66	1833	491	351	351
query67	20286	20113	19962	19962
query68	3318	1627	1027	1027
query69	400	297	265	265
query70	865	798	793	793
query71	390	326	323	323
query72	2981	2617	2294	2294
query73	876	800	410	410
query74	4609	4480	4340	4340
query75	2360	2376	2010	2010
query76	2329	1146	738	738
query77	335	364	279	279
query78	11174	11170	10587	10587
query79	1382	1169	740	740
query80	1270	531	478	478
query81	557	322	279	279
query82	652	169	135	135
query83	380	322	295	295
query84	331	163	131	131
query85	961	608	513	513
query86	412	226	224	224
query87	1971	1964	1847	1847
query88	3733	2799	2794	2794
query89	399	312	298	298
query90	1954	203	188	188
query91	200	192	159	159
query92	65	57	54	54
query93	1791	1637	1053	1053
query94	694	354	335	335
query95	790	575	476	476
query96	1095	835	372	372
query97	2460	2459	2371	2371
query98	195	190	187	187
query99	733	738	610	610
Total cold run time: 245768 ms
Total hot run time: 158519 ms

hello-stephen · 2026-08-10T10:53:12Z

ClickBench: Total hot run time: 23.84 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 8bce8194b7a3a80c4071d7e615c18465fef2355d, data reload: false
query1	0.01	0.00	0.01
query2	0.10	0.05	0.04
query3	0.26	0.13	0.13
query4	1.60	0.14	0.13
query5	0.24	0.22	0.22
query6	1.16	0.84	0.84
query7	0.05	0.01	0.01
query8	0.07	0.04	0.04
query9	0.38	0.32	0.31
query10	0.54	0.56	0.52
query11	0.20	0.13	0.13
query12	0.18	0.14	0.14
query13	0.46	0.46	0.47
query14	0.99	1.01	1.00
query15	0.60	0.58	0.60
query16	0.34	0.31	0.32
query17	1.09	1.09	1.16
query18	0.22	0.21	0.20
query19	1.99	1.93	1.94
query20	0.02	0.01	0.02
query21	15.45	0.21	0.14
query22	4.88	0.05	0.06
query23	16.14	0.29	0.12
query24	2.94	0.40	0.31
query25	0.12	0.04	0.04
query26	0.74	0.20	0.16
query27	0.04	0.03	0.05
query28	3.54	0.79	0.35
query29	12.57	4.06	3.24
query30	0.28	0.16	0.17
query31	2.77	0.56	0.31
query32	3.22	0.59	0.49
query33	3.15	3.12	3.26
query34	15.52	3.98	3.27
query35	3.21	3.20	3.24
query36	0.58	0.43	0.42
query37	0.10	0.06	0.07
query38	0.05	0.03	0.03
query39	0.04	0.04	0.03
query40	0.17	0.15	0.14
query41	0.09	0.03	0.03
query42	0.04	0.02	0.02
query43	0.05	0.04	0.03
Total cold run time: 96.19 s
Total hot run time: 23.84 s

csun5285 · 2026-08-11T01:54:27Z

run buildall

csun5285 · 2026-08-11T01:54:36Z

/review

github-actions

Automated review result: no new blocking findings.

Round 1 converged after three normal full-scope reviews and two separate risk-focused reviews all returned NO_NEW_VALUABLE_FINDINGS. I independently verified their conclusions and completed a final sweep over all 15 changed files and the unresolved-candidate list. No new inline comment is warranted. The existing performance thread about repeated fixed-update key conversion/encoding was treated as known context and was not duplicated.

Critical checkpoint conclusions:

Goal and proof: The PR accomplishes its stated refactor: fixed partial-update loads now run Validate -> FixedPartialUpdateFill -> VariantParse -> RowStoreFill, and both generic writers receive the same full-width logical row. The focused tests cover history/default fill, sequence win/equal/loss outcomes, delete signs, new-row policies, exact TEMP marks/stats, persisted PK bytes, Variant, and row-store rebuilding. The unchanged golden-format test additionally drives two segments through both horizontal and vertical writers, including an omitted sequence column and a combined sequence/Variant/row-store schema, and compares logical cells, raw row-store values, and PK data with pre-refactor output.
Scope and clarity: The change is focused on moving one duplicated fixed-fill implementation into the transform seam, removing the two writer-owned implementations and adding shared conversion helpers plus targeted tests. No unrelated behavior change was found.
Concurrency and lifecycle: Each nonempty memtable receives its final unique segment id before submission; one COW block copy is transformed once, and block-local positions remain final segment row ids. Per-transform state is isolated, flusher counters are atomic, and delete-bitmap access is synchronized. Historical rowsets stay pinned through reads; the row-store generator owns a COW snapshot and is consumed synchronously before its source block dies. No new lock-order, deadlock, static-initialization, ownership-cycle, or leak risk was found.
Data correctness and transactionality: Supplied-sequence probes retain the sequence suffix, omitted-sequence probes remain unsuffixed, and both generic writers build the final PK from the filled sequence. Old-row and losing-new-row TEMP marks, sentinel handling, segcompaction conversion, and publish-time version replacement remain aligned with the prior path. Failures propagate as non-OK status/converted exceptions and terminate the load; no same-block retry can double-apply marks or counters.
Parallel and compatibility paths: Horizontal and vertical fixed updates, flexible updates, grouped row-binlog writes, transient writers, BetaRowsetWriterV2 rejection, schema change, compaction, and multi-segment flushes were traced. No new configuration, FE-BE variable, function/storage format, persistence-log, or rolling-upgrade compatibility change is introduced.
Conditions, errors, and observability: Width/context/segment-id validation still fences unsupported paths before fill. Existing tablet/transaction-aware errors, partial-update stats, delete-bitmap summaries, and logs remain sufficient; no new configuration or metric is needed for this internal refactor.
Performance: The current head invalidates row cache only during final writer key generation. The remaining duplicated key conversion/encoding work is already covered by the existing inline thread, so I did not submit it again. No additional substantiated performance regression was found.
Validation status: Doris compile, clang-format, and style checks are passing for the reviewed head. BE unit tests and regression jobs were still running at review time. Per the review-runner instructions, I did not run builds or tests locally.
User focus: No additional user-provided focus was specified; the complete PR was reviewed.

Overall opinion: the refactor is coherent and well covered, with no new critical blocking issue found. Review completion status: converged in round 1, comment-only review, zero new inline comments.

…pdateFillStage; horizontal writer exits the PU business Fourth PR of the apache#64674 split stack (apache#65492, apache#66151, apache#66528). The fixed partial-update fill moves out of both segment writers into the transform chain: fixed PU loads now run [Validate, FixedPartialUpdateFill, VariantParse, RowStoreFill], so the fill widens the narrow block, probes each key via MowKeyProbe::for_partial_update, reads old rows through a per-block HistoricalRowFetcher, and hands the writers a plain full-width upsert. Flexible partial update keeps its [Validate] bridge and stays in the vertical writer until its own stage lands. - storage/transform/partial_update_fill.{h,cpp}: the fixed fill stage (widen -> stage-local RowKeyEncoder/convertor -> probe_and_plan -> fill_missing_columns -> swap). Fault-injection debug point names are kept so the fault_injection_p0 suites keep working. - storage/transform/transform_util.{h,cpp}: key/seq conversion and the narrow-block widening helper, shared with later stages. - SegmentWriter exits partial update entirely: the fill, its preconditions check, the probe wrapper, the row-store serialization body and the PU stats members are deleted; only the flexible NotSupported rejection stays (removed later as an intended change). - VerticalSegmentWriter drops its fixed fill; write_batch dispatches only flexible loads to the in-writer fill, fixed loads take the regular path; batch_block converges to the single full-width check. - The chain's PartialUpdateStats output returns with its first real producer: transform_block folds the probe counters into the flusher totals; the horizontal _flush_segment_writer stats reads are gone. Equivalence pinned by the golden segment-format tests (apache#65977): PartialUpdateAndRowBinlogPathsKeepTheirSegmentBytes now drives the chain fill and still matches the checked-in bytes. 81 BE UTs green, including 13 new FixedPartialUpdateTest branch-coverage cases and two writer-integration cases that read persisted rowsets and the primary key index back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

csun5285 · 2026-08-11T04:18:04Z

run buildall

hello-stephen · 2026-08-11T07:25:28Z

TPC-H: Total hot run time: 28992 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit b45ad2aede45eb3c823b896a26c7b9f749698630, data reload: false
------ Round 1 ----------------------------------
============================================
q1	17801	3954	3941	3941
q2	2039	312	200	200
q3	10297	1419	843	843
q4	4684	469	334	334
q5	7502	837	543	543
q6	179	172	135	135
q7	758	805	588	588
q8	9666	1588	1686	1588
q9	5930	4101	4110	4101
q10	6753	1612	1375	1375
q11	507	357	325	325
q12	748	570	467	467
q13	18066	3398	2716	2716
q14	258	260	249	249
q15	q16	751	729	666	666
q17	951	985	1015	985
q18	6546	5619	5546	5546
q19	1149	1290	1130	1130
q20	767	668	573	573
q21	5603	2632	2390	2390
q22	426	357	297	297
Total cold run time: 101381 ms
Total hot run time: 28992 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4388	4215	4261	4215
q2	269	315	199	199
q3	4562	4919	4382	4382
q4	2166	2267	1428	1428
q5	4238	4127	4148	4127
q6	237	174	127	127
q7	1723	1600	1476	1476
q8	2637	2207	2104	2104
q9	7235	7264	7225	7225
q10	4304	4549	3968	3968
q11	578	404	359	359
q12	706	728	501	501
q13	3237	3692	2854	2854
q14	283	311	285	285
q15	q16	686	691	650	650
q17	1367	1304	1286	1286
q18	12282	11061	11959	11061
q19	1226	1187	1194	1187
q20	2243	2216	1948	1948
q21	5648	4924	4611	4611
q22	545	458	422	422
Total cold run time: 60560 ms
Total hot run time: 54415 ms

hello-stephen · 2026-08-11T07:35:43Z

TPC-DS: Total hot run time: 158369 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 b45ad2aede45eb3c823b896a26c7b9f749698630, data reload: false
query5	4328	604	458	458
query6	463	216	199	199
query7	4897	622	349	349
query8	320	167	156	156
query9	8768	4101	4058	4058
query10	465	368	316	316
query11	5841	2234	2008	2008
query12	158	100	96	96
query13	1230	599	436	436
query14	6065	4331	3996	3996
query14_1	3862	3790	3774	3774
query15	208	196	180	180
query16	1024	524	493	493
query17	916	704	566	566
query18	2438	477	347	347
query19	206	190	155	155
query20	106	104	103	103
query21	240	161	140	140
query22	13079	13055	12871	12871
query23	16277	14983	14559	14559
query23_1	14686	14666	14778	14666
query24	7506	1727	1240	1240
query24_1	1250	1255	1263	1255
query25	534	431	353	353
query26	1303	339	215	215
query27	2670	620	389	389
query28	4531	1994	1994	1994
query29	1049	597	456	456
query30	347	264	224	224
query31	1186	1120	1054	1054
query32	102	59	60	59
query33	511	310	233	233
query34	1186	1145	628	628
query35	750	736	633	633
query36	789	769	727	727
query37	154	108	91	91
query38	1841	1757	1693	1693
query39	830	824	794	794
query39_1	796	786	779	779
query40	249	162	143	143
query41	70	63	64	63
query42	95	102	92	92
query43	331	337	289	289
query44	1473	771	763	763
query45	192	177	165	165
query46	1040	1181	705	705
query47	1535	1552	1447	1447
query48	408	376	283	283
query49	594	399	305	305
query50	1130	429	347	347
query51	10744	10467	10576	10467
query52	85	87	73	73
query53	267	278	204	204
query54	309	252	235	235
query55	77	75	68	68
query56	335	316	308	308
query57	1048	1011	929	929
query58	288	285	276	276
query59	1587	1647	1430	1430
query60	321	295	267	267
query61	178	176	178	176
query62	405	329	272	272
query63	244	207	207	207
query64	2891	1010	813	813
query65	3879	3811	3845	3811
query66	1836	465	350	350
query67	20065	20219	19975	19975
query68	3161	1499	869	869
query69	416	320	259	259
query70	862	781	787	781
query71	376	339	305	305
query72	2945	2628	2268	2268
query73	833	758	429	429
query74	4659	4496	4307	4307
query75	2365	2327	1992	1992
query76	2315	1170	765	765
query77	345	353	272	272
query78	11169	11094	10697	10697
query79	1361	1195	749	749
query80	1082	536	454	454
query81	496	329	291	291
query82	635	183	140	140
query83	397	325	301	301
query84	329	164	133	133
query85	960	593	513	513
query86	378	233	224	224
query87	2019	1972	1833	1833
query88	3782	2775	2787	2775
query89	400	318	275	275
query90	1794	189	188	188
query91	201	190	160	160
query92	61	60	54	54
query93	1654	1567	966	966
query94	620	357	313	313
query95	787	598	471	471
query96	1094	822	362	362
query97	2489	2460	2329	2329
query98	202	187	183	183
query99	755	741	605	605
Total cold run time: 245753 ms
Total hot run time: 158369 ms

hello-stephen · 2026-08-11T07:40:30Z

ClickBench: Total hot run time: 23.95 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit b45ad2aede45eb3c823b896a26c7b9f749698630, data reload: false
query1	0.00	0.01	0.01
query2	0.10	0.04	0.04
query3	0.25	0.14	0.14
query4	1.61	0.14	0.14
query5	0.24	0.22	0.22
query6	1.17	0.82	0.86
query7	0.04	0.01	0.01
query8	0.05	0.04	0.03
query9	0.38	0.31	0.31
query10	0.57	0.55	0.59
query11	0.20	0.13	0.14
query12	0.19	0.15	0.15
query13	0.48	0.47	0.46
query14	1.02	0.99	0.99
query15	0.61	0.59	0.60
query16	0.31	0.32	0.33
query17	1.12	1.16	1.10
query18	0.21	0.19	0.20
query19	2.10	1.89	1.96
query20	0.02	0.01	0.02
query21	15.43	0.22	0.13
query22	4.74	0.06	0.06
query23	16.15	0.32	0.12
query24	2.96	0.43	0.34
query25	0.13	0.05	0.04
query26	0.73	0.22	0.15
query27	0.03	0.04	0.03
query28	3.52	0.82	0.35
query29	12.53	4.05	3.20
query30	0.27	0.15	0.15
query31	2.78	0.56	0.31
query32	3.22	0.59	0.48
query33	3.17	3.26	3.17
query34	15.55	3.97	3.30
query35	3.27	3.24	3.22
query36	0.55	0.43	0.43
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.18	0.17	0.15
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.22 s
Total hot run time: 23.95 s

liaoxin01

LGTM

github-actions · 2026-08-12T15:13:28Z

PR approved by at least one committer and no changes requested.

hello-stephen · 2026-08-13T02:14:27Z

skip check_coverage

ok

github-actions · 2026-08-13T02:20:28Z

PR approved by anyone and no changes requested.

yiguolei · 2026-08-13T10:49:25Z

/review

github-actions

Review conclusion: no new blocking correctness issue found; approval opinion, submitted as a comment-only review with zero inline comments.

Critical checkpoint conclusions:

Goal and proof: the fixed partial-update fill is moved from both segment writers into [Validate, FixedPartialUpdateFill, VariantParse, RowStoreFill], and the writers receive the same completed full-width row image. The new exact bitmap/stats/error/readback tests and the existing horizontal/vertical multi-segment golden-format cases cover the principal behavior.
Scope and focus: the production change is focused on the transform/writer ownership seam. I reviewed all 15 changed files and the upstream/downstream probe, historical-read, rowset-flush, row-binlog, and primary-key-index paths. No additional user focus was supplied.
Concurrency: concurrent memtable flushes use per-flush transform state; flusher totals are atomic and delete-bitmap access is internally locked. No new lock ordering, deadlock, or unsafe shared lifetime was found.
Lifecycle: HistoricalRowFetcher pins source rowsets through the read, and the RowStore generator owns a COW snapshot until horizontal materialization or vertical bounded generation completes. No cycle, cross-TU static dependency, or premature release was found.
Configuration: no configuration item was added. Both horizontal and vertical writer selections retain fixed-update behavior; flexible updates remain vertical-only.
Compatibility and parallel paths: no protocol or storage-format change was introduced. Normal/cloud flushes traverse the new fixed fill once; transient, row-binlog, compaction, schema-change, and flexible-update routes neither bypass a required fill nor apply it twice.
Conditions and errors: segment-id and block-width fences fail explicitly; the fixed path preserves supplied/omitted sequence, delete-sign, new-key, default/null, required-column, and auto-increment behavior. Status and exception boundaries remain checked.
Data-write correctness: the transform receives the final segment id, starts at physical row 0, and preserves row order/count, so old-row and losing-new-row TEMP delete-bitmap marks retain the correct {rowset, segment, row} identity. A later writer failure fails the flush token/transaction, preventing publication of the failed rowset and its counters.
Tests and expected results: branch tests assert exact row images, bitmap membership, counters, error policies, sentinel marks, row-store reads, persisted rows, and key encodings. Existing format tests cover two segments, both writer modes, sequence supplied/omitted, Variant, and RowStore. Per runner instructions this was a review-only pass; no build or test command was run.
Observability and persistence: no new persistent metadata, EditLog, FE/BE variable, or configuration propagation is involved. Existing contextual errors and flush logging remain sufficient for this refactor.
Performance: the extra fixed-update key conversion/encoding work is already covered by existing discussions r3747875632 and r3763226452 and was not duplicated. I found no additional substantiated performance issue.
Other issues: none found after the main full-diff sweep, two normal full-review scopes, a separate five-risk seam review, and a final changed-file/unresolved-candidate sweep. All Round 1 agents returned NO_NEW_VALUABLE_FINDINGS, so the review converged in one round.

…pdateFillStage (apache#66609) Fourth PR of the apache#64674 split stack (apache#65492, apache#66151, apache#66528). The fixed partial-update fill moves out of both segment writers into the transform chain: fixed PU loads now run [Validate, FixedPartialUpdateFill, VariantParse, RowStoreFill], so the fill widens the narrow block, probes each key via MowKeyProbe::for_partial_update, reads old rows through a per-block HistoricalRowFetcher, and hands the writers a plain full-width upsert. ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test  - [ ] Regression test - [x] Unit Test - [ ] 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  - Behavior changed: - [ ] No. - [ ] Yes.  - 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  Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

…RowBinlogSegmentWriter (#66883) ### What problem does this PR solve? Related PR: #66774 (stack PR 5/6), #66609 (4/6), #66528 (3/6), #66151 (2/6), #65492 (1/6) Last PR of the block-transform-chain refactor stack, and the one that pays it off: with the binlog derivation in the chain, no segment writer derives or fills anything any more -- both writers are pure storage-format sinks. **`RowBinlogDeriveStage`** : rebuilds the load block into a full-width block over the binlog schema -- key + AFTER values, optional `__BEFORE__*` values, and the TSO / LSN / op columns ### Release note None ### Check List (For Author) - Test  - [ ] Regression test - [ ] Unit Test - [ ] 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  - Behavior changed: - [ ] No. - [ ] Yes.  - 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  --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

csun5285 requested review from airborne12, eldenmoon, gavinchou, liaoxin01 and yiguolei as code owners August 10, 2026 07:54

csun5285 force-pushed the refactor/fixed-pu-fill-stage branch from b946f17 to 8bce819Compare August 10, 2026 08:00

github-actionsBot previously requested changes Aug 10, 2026
View reviewed changes

Comment threadbe/src/storage/transform/partial_update_fill.cpp

github-actionsBot reviewed Aug 10, 2026
View reviewed changes

csun5285 force-pushed the refactor/fixed-pu-fill-stage branch from 8bce819 to 5468666Compare August 11, 2026 01:51

github-actionsBot reviewed Aug 11, 2026
View reviewed changes

csun5285 force-pushed the refactor/fixed-pu-fill-stage branch from 5468666 to b45ad2aCompare August 11, 2026 04:16

bobhan1 reviewed Aug 12, 2026
View reviewed changes

Comment threadbe/src/storage/transform/partial_update_fill.cpp

bobhan1 approved these changes Aug 12, 2026
View reviewed changes

liaoxin01 approved these changes Aug 12, 2026
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Aug 12, 2026

github-actionsBot added the reviewed label Aug 13, 2026

github-actionsBot reviewed Aug 13, 2026
View reviewed changes

yiguolei approved these changes Aug 14, 2026
View reviewed changes

yiguolei merged commit 6eed32e into apache:masterAug 14, 2026
35 checks passed

csun5285 mentioned this pull request Aug 18, 2026
[refactor](storage) derive row binlog in the transform chain; delete RowBinlogSegmentWriter #66883
Merged
16 tasks

Uh oh!

Conversation

csun5285 commented Aug 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Aug 10, 2026

Uh oh!

csun5285 commented Aug 10, 2026

Uh oh!

csun5285 commented Aug 10, 2026

Uh oh!

csun5285 commented Aug 10, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Aug 10, 2026

Uh oh!

hello-stephen commented Aug 10, 2026

Uh oh!

hello-stephen commented Aug 10, 2026

Uh oh!

csun5285 commented Aug 11, 2026

Uh oh!

csun5285 commented Aug 11, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

csun5285 commented Aug 11, 2026

Uh oh!

hello-stephen commented Aug 11, 2026

Uh oh!

hello-stephen commented Aug 11, 2026

Uh oh!

hello-stephen commented Aug 11, 2026

Uh oh!

Uh oh!

liaoxin01 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsBot commented Aug 12, 2026

Uh oh!

hello-stephen commented Aug 13, 2026

Uh oh!

github-actionsBot commented Aug 13, 2026

Uh oh!

yiguolei commented Aug 13, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

csun5285 commented Aug 10, 2026•
edited
Loading