Skip to content

[Enhance](func) Delete useless code of split_by_char#64801

Merged
924060929 merged 1 commit into
apache:masterfrom
linrrzqqq:drop-split-by-char-fe
Jun 25, 2026
Merged

[Enhance](func) Delete useless code of split_by_char#64801
924060929 merged 1 commit into
apache:masterfrom
linrrzqqq:drop-split-by-char-fe

Conversation

@linrrzqqq

@linrrzqqq linrrzqqq commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Release note

There is a function registration part for SPLIT_BY_CHAR in FE, but there is no corresponding implementation in BE.

Doris> SELECT SPLIT_BY_CHAR('apple:banana:cherry', ':');
-- ERROR 1105 (HY000): errCode = 2, detailMessage = (127.0.0.1)[INTERNAL_ERROR]Could not find function split_by_char, arg String: String, String: String return Array(Nullable(String)) 

Since the functionality of this function has already been implemented by SPLIT_BY_STRING, the implementation of this function is not meaningful and can be directly deleted.

@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?

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

run buildall

@linrrzqqq

Copy link
Copy Markdown
Collaborator 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.

Part 1.3 checkpoint conclusions:

  • Goal/test proof: The PR removes the Nereids-only split_by_char wrapper and registration. This matches the stated goal because BE has no split_by_char implementation, while split_by_string remains registered and covered by existing regression tests. No new runtime behavior test is necessary for deleting an unsupported FE-only symbol.
  • Scope/focus: The change is small and focused on the obsolete function metadata plus its visitor hook.
  • Concurrency/lifecycle/config/compatibility: No concurrency, lifecycle ownership, session/config propagation, thrift/FE-BE protocol, or storage compatibility paths are touched. The compatibility-relevant observation is that the removed symbol was not executable in BE.
  • Parallel paths: Current references show split_by_string/split remain registered in FE and BE; no remaining SplitByChar, split_by_char, or visitSplitByChar code path remains except an unrelated regression table name.
  • Tests/style: Existing split-by-string regression coverage remains in place. Scoped git diff --check over the PR files reported no whitespace/style issues. Full FE/regression CI was not run in this review.
  • Other risks: No optimizer rewrite, join/aggregate, data visibility, transaction, persistence, observability, or memory-safety behavior is changed by this deletion.

User focus: No additional user-provided review focus was supplied.

Subagent conclusions:

  • optimizer-rewrite: no candidates; confirmed the PR does not affect optimizer/rewrite, join, aggregate, or parallel execution paths.
  • tests-session-config: no candidates; confirmed no regression output, session/config propagation, compatibility, or scoped style issue worth raising.
  • Convergence: Round 1 ended with both live subagents reporting NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set after the final candidate update.

Overall, I found no blocking issue or inline comment to submit.

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17650	4007	3965	3965
q2	2028	303	190	190
q3	10285	1345	833	833
q4	4681	462	342	342
q5	7490	841	585	585
q6	188	168	138	138
q7	744	861	626	626
q8	9447	1295	1505	1295
q9	6073	4495	4490	4490
q10	6792	1826	1534	1534
q11	453	275	243	243
q12	636	421	293	293
q13	18089	3350	2715	2715
q14	277	259	232	232
q15	q16	785	785	718	718
q17	956	874	853	853
q18	6961	5622	5493	5493
q19	1297	1336	1044	1044
q20	504	399	262	262
q21	6029	2508	2494	2494
q22	439	359	292	292
Total cold run time: 101804 ms
Total hot run time: 28637 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4305	4249	4283	4249
q2	315	337	217	217
q3	4563	4948	4374	4374
q4	2036	2180	1387	1387
q5	4437	4269	4304	4269
q6	238	175	130	130
q7	1710	1609	1466	1466
q8	2873	2178	2173	2173
q9	8068	8122	8093	8093
q10	4800	4735	4328	4328
q11	569	414	397	397
q12	749	745	545	545
q13	3220	3624	2941	2941
q14	310	306	295	295
q15	q16	743	726	640	640
q17	1383	1358	1324	1324
q18	7977	7155	7096	7096
q19	1150	1090	1097	1090
q20	2241	2230	1992	1992
q21	5246	4579	4390	4390
q22	510	469	418	418
Total cold run time: 57443 ms
Total hot run time: 51814 ms

@hello-stephen

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

query5	4322	640	466	466
query6	448	191	166	166
query7	4882	542	299	299
query8	357	212	206	206
query9	8735	4068	4035	4035
query10	445	316	254	254
query11	5721	2305	2128	2128
query12	156	102	101	101
query13	1263	612	421	421
query14	6380	5377	5044	5044
query14_1	4391	4357	4367	4357
query15	209	198	181	181
query16	1012	453	431	431
query17	1196	730	578	578
query18	2649	482	375	375
query19	214	187	145	145
query20	116	108	104	104
query21	221	145	119	119
query22	13566	13662	13381	13381
query23	17332	16495	16105	16105
query23_1	16282	16196	16342	16196
query24	7707	1785	1304	1304
query24_1	1327	1304	1287	1287
query25	555	439	369	369
query26	1171	322	171	171
query27	2623	593	345	345
query28	4453	2002	2011	2002
query29	1075	611	482	482
query30	314	223	198	198
query31	1110	1059	968	968
query32	103	67	65	65
query33	504	306	237	237
query34	1190	1123	655	655
query35	748	809	683	683
query36	1355	1372	1252	1252
query37	151	107	92	92
query38	1885	1728	1630	1630
query39	933	919	893	893
query39_1	880	869	877	869
query40	211	121	96	96
query41	64	61	60	60
query42	85	84	82	82
query43	322	319	288	288
query44	1464	782	772	772
query45	191	186	173	173
query46	1061	1173	783	783
query47	2316	2394	2224	2224
query48	404	401	291	291
query49	613	459	346	346
query50	969	359	255	255
query51	4533	4308	4331	4308
query52	86	83	77	77
query53	268	277	196	196
query54	268	211	206	206
query55	73	70	66	66
query56	228	220	203	203
query57	1435	1394	1323	1323
query58	243	206	201	201
query59	1540	1645	1404	1404
query60	273	242	223	223
query61	148	141	145	141
query62	706	649	584	584
query63	225	192	191	191
query64	2325	738	582	582
query65	4876	4796	4744	4744
query66	1704	453	342	342
query67	29640	29589	29510	29510
query68	3059	1616	999	999
query69	416	303	279	279
query70	1077	996	982	982
query71	278	222	212	212
query72	2936	2582	2302	2302
query73	849	817	446	446
query74	5130	4921	4784	4784
query75	2618	2573	2230	2230
query76	2314	1196	795	795
query77	345	371	286	286
query78	12285	12442	11868	11868
query79	1465	1140	736	736
query80	658	470	383	383
query81	471	284	241	241
query82	578	165	122	122
query83	321	273	247	247
query84	256	141	117	117
query85	903	500	410	410
query86	424	302	278	278
query87	1840	1824	1742	1742
query88	3688	2779	2766	2766
query89	426	370	335	335
query90	1862	182	176	176
query91	167	159	134	134
query92	65	61	54	54
query93	1559	1562	973	973
query94	580	358	292	292
query95	689	463	350	350
query96	1132	842	351	351
query97	2749	2718	2583	2583
query98	226	199	211	199
query99	1184	1188	1028	1028
Total cold run time: 257272 ms
Total hot run time: 172469 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.11	0.05	0.05
query3	0.26	0.14	0.14
query4	1.62	0.14	0.14
query5	0.24	0.22	0.22
query6	1.23	1.09	1.08
query7	0.04	0.01	0.01
query8	0.08	0.04	0.04
query9	0.38	0.31	0.32
query10	0.55	0.55	0.54
query11	0.21	0.14	0.14
query12	0.19	0.16	0.15
query13	0.47	0.48	0.47
query14	1.01	0.98	1.01
query15	0.62	0.59	0.60
query16	0.30	0.34	0.32
query17	1.13	1.14	1.09
query18	0.23	0.22	0.21
query19	2.04	1.91	1.99
query20	0.02	0.01	0.01
query21	15.45	0.19	0.13
query22	4.99	0.05	0.05
query23	16.11	0.32	0.12
query24	2.98	0.43	0.32
query25	0.14	0.06	0.05
query26	0.72	0.21	0.16
query27	0.05	0.04	0.04
query28	3.51	0.91	0.54
query29	12.54	4.29	3.45
query30	0.28	0.15	0.17
query31	2.77	0.59	0.31
query32	3.22	0.59	0.49
query33	3.23	3.17	3.26
query34	15.48	4.25	3.60
query35	3.50	3.54	3.53
query36	0.56	0.44	0.42
query37	0.09	0.07	0.06
query38	0.06	0.04	0.04
query39	0.03	0.03	0.03
query40	0.18	0.16	0.15
query41	0.09	0.03	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 96.81 s
Total hot run time: 25.31 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage `` 🎉
Increment coverage report
Complete coverage report

@HappenLee HappenLee 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

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 24, 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.

@924060929 924060929 merged commit 1097d51 into apache:master Jun 25, 2026
35 of 36 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 25, 2026
### Release note

There is a function registration part for `SPLIT_BY_CHAR` in FE, but
there is no corresponding implementation in BE.

```sql
Doris> SELECT SPLIT_BY_CHAR('apple:banana:cherry', ':');
-- ERROR 1105 (HY000): errCode = 2, detailMessage = (127.0.0.1)[INTERNAL_ERROR]Could not find function split_by_char, arg String: String, String: String return Array(Nullable(String)) 
``` 

Since the functionality of this function has already been implemented by
`SPLIT_BY_STRING`, the implementation of this function is not meaningful
and can be directly deleted.
github-actions Bot pushed a commit that referenced this pull request Jun 25, 2026
### Release note

There is a function registration part for `SPLIT_BY_CHAR` in FE, but
there is no corresponding implementation in BE.

```sql
Doris> SELECT SPLIT_BY_CHAR('apple:banana:cherry', ':');
-- ERROR 1105 (HY000): errCode = 2, detailMessage = (127.0.0.1)[INTERNAL_ERROR]Could not find function split_by_char, arg String: String, String: String return Array(Nullable(String)) 
``` 

Since the functionality of this function has already been implemented by
`SPLIT_BY_STRING`, the implementation of this function is not meaningful
and can be directly deleted.
@linrrzqqq linrrzqqq deleted the drop-split-by-char-fe branch June 25, 2026 03:19
yiguolei pushed a commit that referenced this pull request Jun 25, 2026
… (#64815)

Cherry-picked from #64801

Co-authored-by: linrrarity <linzhenqi@selectdb.com>
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.1.3-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants