Skip to content

[opt](serde)Build Arrow serde error context lazily.#64660

Merged
Gabriel39 merged 1 commit into
apache:masterfrom
hubgeter:opt_arrow_parquet_write
Jun 23, 2026
Merged

[opt](serde)Build Arrow serde error context lazily.#64660
Gabriel39 merged 1 commit into
apache:masterfrom
hubgeter:opt_arrow_parquet_write

Conversation

@hubgeter

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Problem Summary:
Optimize the arrow serde checkArrowStatus method.

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

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?

@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@Gabriel39

Copy link
Copy Markdown
Contributor

/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 PR 64660 against the Doris code-review checklist and the BE core AGENTS guidance. I did not find a substantiated code issue requiring an inline comment.

Critical checkpoint conclusions:

  • Goal/test evidence: the PR narrows checkArrowStatus so Arrow type and column names are read only on error; all updated call sites preserve the same Arrow append/null-map/value operations. Existing Arrow serde coverage is the relevant coverage; I did not run a local BE build or UT.
  • Scope: the change is small and focused on BE Arrow serde error-context construction.
  • Concurrency/lifecycle: no new shared state, locking, threads, or static initialization dependency.
  • Config/session/compatibility/persistence: no new config, session propagation, protocol field, serialization layout, storage format, or FE/BE compatibility change.
  • Parallel paths: the changed-file list matches the PR diff, and the data_type_serde checkArrowStatus call sites were updated consistently. Nullable, array/map/struct nested forwarding and upstream convert_to_arrow_batch status propagation remain unchanged.
  • Error handling: the helper still returns Status::FatalError only when Arrow returns non-OK, and upstream callers continue to propagate/prepend context through the existing RETURN_IF_ERROR paths.
  • Memory/COW/nullability: no changed ownership, column mutation, or null-map semantics.
  • Style/CI: git diff --check is clean. GitHub Clang Formatter and CheckStyle checks are passing. The visible BE UT (macOS) failure is an environment setup failure before compilation/tests (JAVA version is 25, it must be JDK-17), so I did not treat it as a PR code defect.

User focus: no additional user-provided review focus was present.

Subagent conclusions: optimizer-rewrite reported no optimizer/rewrite, semantic-equivalence, FE/BE type-consistency, or parallel join/aggregate issue. tests-session-config reported no regression-test, expected-output, session/config, compatibility, or basic style/CI issue. After the main ledger update, convergence round 1 ended with both subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set.

No inline review comments submitted because no candidate finding was accepted.

@yx-keith yx-keith 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.

lgtm

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 81.03% (47/58) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.21% (28440/38326)
Line Coverage 58.11% (310113/533652)
Region Coverage 54.94% (259810/472863)
Branch Coverage 56.24% (112727/200452)

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17728	4107	4040	4040
q2	2032	332	191	191
q3	10267	1453	809	809
q4	4696	472	339	339
q5	7500	854	564	564
q6	186	167	139	139
q7	766	852	621	621
q8	10100	1607	1699	1607
q9	6248	4550	4568	4550
q10	6823	1795	1522	1522
q11	440	275	246	246
q12	628	417	297	297
q13	18111	3364	2752	2752
q14	268	260	244	244
q15	q16	780	782	716	716
q17	1025	1004	1018	1004
q18	6875	5743	5534	5534
q19	1180	1322	1133	1133
q20	474	412	272	272
q21	5607	2789	2459	2459
q22	440	356	302	302
Total cold run time: 102174 ms
Total hot run time: 29341 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4393	4309	4284	4284
q2	333	360	224	224
q3	4631	4956	4369	4369
q4	2058	2159	1372	1372
q5	4420	4326	4326	4326
q6	228	171	129	129
q7	1728	2107	1718	1718
q8	2501	2138	2102	2102
q9	8042	7958	7919	7919
q10	4783	4753	4366	4366
q11	616	406	394	394
q12	854	828	538	538
q13	3269	3511	2947	2947
q14	307	313	275	275
q15	q16	745	756	656	656
q17	1359	1328	1308	1308
q18	8053	7341	6873	6873
q19	1104	1142	1098	1098
q20	2248	2225	1955	1955
q21	5260	4587	4411	4411
q22	521	462	411	411
Total cold run time: 57453 ms
Total hot run time: 51675 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 172883 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 c89c0e85b68399867ed1de7ca978c14fd627450b, data reload: false

query5	4345	624	492	492
query6	435	192	172	172
query7	4858	548	301	301
query8	364	215	195	195
query9	8780	4045	4044	4044
query10	444	319	257	257
query11	5815	2333	2109	2109
query12	151	103	100	100
query13	1245	614	446	446
query14	6374	5379	5073	5073
query14_1	4362	4350	4386	4350
query15	208	200	173	173
query16	999	459	467	459
query17	1002	694	548	548
query18	2427	477	338	338
query19	197	181	143	143
query20	113	112	115	112
query21	217	136	119	119
query22	13656	13642	13383	13383
query23	17415	16600	16039	16039
query23_1	16195	16202	16204	16202
query24	7487	1755	1307	1307
query24_1	1320	1304	1319	1304
query25	537	443	397	397
query26	1300	328	169	169
query27	2769	551	352	352
query28	4509	2056	2024	2024
query29	1086	605	477	477
query30	321	234	199	199
query31	1101	1068	955	955
query32	107	60	59	59
query33	515	309	244	244
query34	1180	1103	647	647
query35	748	774	690	690
query36	1377	1398	1250	1250
query37	153	105	90	90
query38	1895	1719	1652	1652
query39	932	926	904	904
query39_1	869	882	869	869
query40	228	130	108	108
query41	72	67	67	67
query42	89	87	92	87
query43	326	328	279	279
query44	1454	796	797	796
query45	200	188	179	179
query46	1024	1175	740	740
query47	2413	2343	2276	2276
query48	420	428	306	306
query49	653	480	364	364
query50	1028	351	257	257
query51	4325	4315	4275	4275
query52	88	87	72	72
query53	252	264	196	196
query54	289	232	209	209
query55	78	76	67	67
query56	270	225	243	225
query57	1448	1445	1315	1315
query58	256	223	222	222
query59	1565	1680	1445	1445
query60	287	261	241	241
query61	178	172	182	172
query62	684	651	596	596
query63	242	189	198	189
query64	2588	832	719	719
query65	4884	4806	4774	4774
query66	1785	457	333	333
query67	29972	29759	29598	29598
query68	3231	1518	992	992
query69	415	294	271	271
query70	1058	996	960	960
query71	290	229	197	197
query72	2907	2580	2322	2322
query73	831	772	446	446
query74	5106	5015	4757	4757
query75	2629	2609	2222	2222
query76	2327	1196	791	791
query77	358	383	284	284
query78	12449	12483	12015	12015
query79	1407	1203	761	761
query80	1273	462	378	378
query81	530	286	244	244
query82	668	153	127	127
query83	333	292	245	245
query84	297	142	117	117
query85	931	524	423	423
query86	442	320	281	281
query87	1832	1822	1788	1788
query88	3722	2809	2785	2785
query89	426	377	328	328
query90	1929	194	181	181
query91	174	159	139	139
query92	67	62	57	57
query93	1627	1525	975	975
query94	772	353	319	319
query95	693	366	452	366
query96	1141	804	356	356
query97	2702	2678	2560	2560
query98	211	210	198	198
query99	1178	1180	1033	1033
Total cold run time: 259172 ms
Total hot run time: 172883 ms

@hello-stephen

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

query1	0.00	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.14	0.14
query4	1.61	0.14	0.14
query5	0.24	0.22	0.22
query6	1.27	1.04	1.03
query7	0.04	0.01	0.00
query8	0.06	0.04	0.04
query9	0.41	0.30	0.33
query10	0.54	0.54	0.57
query11	0.20	0.14	0.14
query12	0.19	0.14	0.14
query13	0.47	0.47	0.47
query14	1.01	1.02	1.00
query15	0.61	0.58	0.60
query16	0.31	0.32	0.33
query17	1.09	1.06	1.08
query18	0.24	0.22	0.22
query19	1.99	1.97	1.98
query20	0.02	0.01	0.01
query21	15.42	0.25	0.14
query22	4.76	0.05	0.05
query23	16.11	0.31	0.12
query24	3.06	0.40	0.34
query25	0.12	0.05	0.04
query26	0.73	0.21	0.17
query27	0.03	0.04	0.05
query28	3.50	0.96	0.54
query29	12.49	4.32	3.50
query30	0.27	0.15	0.15
query31	2.78	0.60	0.31
query32	3.22	0.60	0.49
query33	3.23	3.30	3.35
query34	15.58	4.18	3.51
query35	3.51	3.51	3.51
query36	0.55	0.43	0.42
query37	0.10	0.07	0.07
query38	0.05	0.04	0.03
query39	0.04	0.03	0.03
query40	0.17	0.15	0.16
query41	0.09	0.03	0.03
query42	0.04	0.02	0.02
query43	0.04	0.04	0.03
Total cold run time: 96.54 s
Total hot run time: 25.36 s

@Gabriel39 Gabriel39 merged commit 17867b8 into apache:master Jun 23, 2026
34 of 35 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 23, 2026
Optimize the arrow serde `checkArrowStatus` method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.0.x dev/4.0.x-conflict dev/4.1.x reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants