Skip to content

[fix](be) Validate dictionary page codes - #66832

Open
YanzhiJin5 wants to merge 1 commit into
apache:masterfrom
YanzhiJin5:fix-dictionary-page-codes
Open

[fix](be) Validate dictionary page codes#66832
YanzhiJin5 wants to merge 1 commit into
apache:masterfrom
YanzhiJin5:fix-dictionary-page-codes

Conversation

@YanzhiJin5

Copy link
Copy Markdown

What problem does this PR solve?

Issue Number: #63609

Related PR: N/A

Problem Summary:

Persisted dictionary pages may contain invalid signed int32_t dictionary codes. BinaryDictPageDecoder previously allowed negative or out-of-range codes to reach downstream dictionary handling without enforcing:

0 <= code < dictionary_size

This could expose invalid dictionary state to downstream consumers instead of returning a corruption error at the storage decoding boundary.

This PR validates decoded dictionary codes before they become caller-visible:

  • next_batch() validates the complete decoded batch before inserting normal dictionary output, resolving only_read_offsets, updating *n, or advancing the decoder cursor.
  • read_by_rowids() gathers and validates all selected codes before producing caller-visible output or updating *n.
  • Negative codes and codes greater than or equal to the dictionary size now return Status::Corruption.
  • The validation uses a signedness-safe bounds check.
  • Valid dictionary-code behavior remains unchanged.

The regression test covers:

  • negative dictionary code (-1);
  • dictionary code equal to dictionary_size;
  • next_batch();
  • read_by_rowids();
  • normal dictionary output;
  • only_read_offsets;
  • unchanged destination/count/cursor state when corruption is detected;
  • valid-input behavior for all covered paths.

The regression test was verified RED → GREEN: it fails without the production validation and passes after the validation is restored.

ASAN unit test:

BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run -j2 \
  --filter='BinaryDictPageTest.RejectInvalidDictionaryCodes'

Result: PASS, 1/1.

Additional checks:

  • build-support/clang-format.sh: PASS
  • build-support/check-format.sh: PASS
  • git diff --check origin/master...HEAD: PASS
  • clang-tidy was attempted but was inconclusive due to pre-existing/toolchain diagnostics; no new diagnostic was observed on the newly added validation or regression-test lines.

The validation adds one additional sequential O(n) pass over decoded dictionary codes for valid data. No directly applicable BinaryDictPageDecoder benchmark is currently available, so the performance impact has not been quantitatively established.

This addresses a matching dictionary-decoder robustness issue found while investigating the reported crash. The original production segment was not available for exact reproduction, so this PR does not claim reproduction of the exact production crash.

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. Invalid persisted dictionary codes are now rejected with Status::Corruption at the decoder boundary instead of being propagated to downstream dictionary handling.
  • 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?

@YanzhiJin5

Copy link
Copy Markdown
Author

run buildall

@YanzhiJin5

Copy link
Copy Markdown
Author

/review

@YanzhiJin5
YanzhiJin5 force-pushed the fix-dictionary-page-codes branch from ade5eba to ff369e0 Compare August 18, 2026 23:56
@YanzhiJin5

Copy link
Copy Markdown
Author

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	18013	3057	3059	3057
q2	2255	235	156	156
q3	10312	896	529	529
q4	4724	251	205	205
q5	7620	561	396	396
q6	134	113	92	92
q7	531	505	391	391
q8	9309	893	956	893
q9	3813	2391	2382	2382
q10	6591	830	706	706
q11	688	280	243	243
q12	790	383	327	327
q13	18015	1546	1174	1174
q14	156	149	138	138
q15	q16	439	408	366	366
q17	816	797	810	797
q18	3190	2269	2213	2213
q19	1273	893	833	833
q20	660	543	442	442
q21	5334	1685	1829	1685
q22	320	261	227	227
Total cold run time: 94983 ms
Total hot run time: 17252 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3430	3362	3341	3341
q2	211	211	161	161
q3	2169	2361	2114	2114
q4	1187	1156	891	891
q5	2184	2114	2107	2107
q6	177	134	87	87
q7	991	933	846	846
q8	1614	1424	1411	1411
q9	3107	3095	3103	3095
q10	1849	1783	1622	1622
q11	350	277	266	266
q12	461	430	335	335
q13	1474	1519	1155	1155
q14	172	187	174	174
q15	q16	391	388	353	353
q17	1058	1042	1033	1033
q18	4965	4405	4697	4405
q19	936	862	839	839
q20	1020	1000	804	804
q21	3598	2984	3205	2984
q22	389	341	316	316
Total cold run time: 31733 ms
Total hot run time: 28339 ms

@hello-stephen

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

query5	4248	414	342	342
query6	449	168	160	160
query7	4939	462	248	248
query8	723	125	116	116
query9	8679	2941	2912	2912
query10	462	241	218	218
query11	5378	1048	930	930
query12	128	74	73	73
query13	1232	451	337	337
query14	6051	2203	2078	2078
query14_1	1965	1978	1961	1961
query15	177	122	115	115
query16	3202	399	365	365
query17	808	469	408	408
query18	2200	330	249	249
query19	229	141	118	118
query20	72	74	70	70
query21	863	122	102	102
query22	5737	5342	5352	5342
query23	6963	6160	6020	6020
query23_1	5874	5869	6101	5869
query24	7334	1100	778	778
query24_1	777	782	811	782
query25	411	282	234	234
query26	1267	257	157	157
query27	2694	460	295	295
query28	4555	1499	1484	1484
query29	997	456	355	355
query30	429	177	159	159
query31	908	439	379	379
query32	102	49	47	47
query33	514	214	174	174
query34	1014	845	516	516
query35	422	397	342	342
query36	571	558	539	539
query37	127	79	79	79
query38	1040	837	810	810
query39	515	479	487	479
query39_1	455	490	483	483
query40	260	134	110	110
query41	64	48	50	48
query42	75	77	85	77
query43	243	245	220	220
query44	1047	554	559	554
query45	110	109	97	97
query46	798	856	540	540
query47	771	748	720	720
query48	325	381	235	235
query49	641	234	190	190
query50	781	338	281	281
query51	8146	8084	8045	8045
query52	70	74	64	64
query53	195	212	154	154
query54	236	175	164	164
query55	79	56	53	53
query56	221	228	213	213
query57	986	653	636	636
query58	225	209	190	190
query59	1194	1207	1082	1082
query60	282	209	193	193
query61	133	112	142	112
query62	397	200	179	179
query63	188	150	148	148
query64	2310	701	582	582
query65	1598	1561	1589	1561
query66	1767	286	249	249
query67	9721	9684	9658	9658
query68	2768	1169	762	762
query69	545	240	192	192
query70	675	616	640	616
query71	298	277	247	247
query72	2662	1737	1607	1607
query73	700	630	343	343
query74	1579	1247	1139	1139
query75	1234	1174	1020	1020
query76	1872	728	552	552
query77	261	271	228	228
query78	3823	3625	3119	3119
query79	1401	800	577	577
query80	1154	429	383	383
query81	536	200	173	173
query82	661	133	107	107
query83	336	264	239	239
query84	317	130	107	107
query85	944	483	429	429
query86	411	172	166	166
query87	992	967	882	882
query88	2837	2125	2148	2125
query89	329	226	196	196
query90	1986	142	142	142
query91	160	140	127	127
query92	54	47	45	45
query93	1423	1133	783	783
query94	636	261	224	224
query95	631	443	338	338
query96	837	563	269	269
query97	1068	1035	971	971
query98	143	133	131	131
query99	567	351	307	307
Total cold run time: 185938 ms
Total hot run time: 83206 ms

@hello-stephen

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

query1	0.00	0.00	0.00
query2	0.34	0.03	0.03
query3	0.55	0.11	0.11
query4	1.90	0.10	0.10
query5	0.17	0.15	0.16
query6	1.47	0.67	0.68
query7	0.03	0.01	0.00
query8	0.04	0.02	0.03
query9	0.59	0.21	0.22
query10	0.36	0.34	0.34
query11	0.28	0.11	0.12
query12	0.22	0.13	0.12
query13	0.30	0.32	0.31
query14	0.46	0.45	0.45
query15	0.38	0.35	0.34
query16	0.24	0.22	0.23
query17	0.70	0.70	0.68
query18	0.16	0.16	0.17
query19	1.50	1.18	1.22
query20	0.02	0.01	0.01
query21	15.86	0.15	0.12
query22	4.78	0.04	0.04
query23	16.57	0.25	0.10
query24	13.60	0.33	0.28
query25	0.11	0.06	0.04
query26	0.73	0.17	0.13
query27	0.04	0.04	0.02
query28	3.69	0.59	0.27
query29	12.56	3.16	2.55
query30	0.28	0.10	0.12
query31	2.82	0.36	0.17
query32	3.77	0.32	0.23
query33	1.35	1.38	1.38
query34	15.33	2.12	1.74
query35	1.73	1.71	1.69
query36	0.46	0.30	0.30
query37	0.20	0.04	0.04
query38	0.05	0.04	0.03
query39	0.13	0.02	0.02
query40	0.15	0.07	0.07
query41	0.36	0.02	0.02
query42	0.12	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 104.43 s
Total hot run time: 14.6 s

@YanzhiJin5

Copy link
Copy Markdown
Author

run beut

@YanzhiJin5

Copy link
Copy Markdown
Author

/review

@YanzhiJin5
YanzhiJin5 force-pushed the fix-dictionary-page-codes branch from ff369e0 to 0966d65 Compare August 20, 2026 04:48
@YanzhiJin5

Copy link
Copy Markdown
Author

run buildall

@YanzhiJin5

Copy link
Copy Markdown
Author

/review

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17619	3113	3101	3101
q2	1952	250	157	157
q3	10390	881	501	501
q4	4666	250	219	219
q5	7662	566	389	389
q6	138	117	91	91
q7	523	501	386	386
q8	9242	923	890	890
q9	3492	2428	2402	2402
q10	6529	863	704	704
q11	441	254	246	246
q12	696	389	323	323
q13	17860	1532	1149	1149
q14	159	154	135	135
q15	q16	424	394	360	360
q17	824	807	791	791
q18	3107	2262	2208	2208
q19	1119	914	790	790
q20	654	518	442	442
q21	5211	1890	1904	1890
q22	325	268	232	232
Total cold run time: 93033 ms
Total hot run time: 17406 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3472	3415	3395	3395
q2	213	214	154	154
q3	2299	2360	2191	2191
q4	1191	1165	900	900
q5	2163	2113	2125	2113
q6	175	121	87	87
q7	1019	903	844	844
q8	1634	1440	1425	1425
q9	3157	3111	3103	3103
q10	1831	1830	1667	1667
q11	356	273	252	252
q12	459	431	339	339
q13	1474	1527	1164	1164
q14	168	176	176	176
q15	q16	394	398	355	355
q17	1063	1053	1044	1044
q18	4921	4387	4857	4387
q19	889	855	841	841
q20	978	942	788	788
q21	3812	3159	3366	3159
q22	423	365	333	333
Total cold run time: 32091 ms
Total hot run time: 28717 ms

@hello-stephen

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

query5	4257	431	343	343
query6	416	186	155	155
query7	4823	451	273	273
query8	305	127	119	119
query9	8693	2929	2900	2900
query10	414	254	245	245
query11	5392	1054	922	922
query12	128	72	77	72
query13	1223	436	334	334
query14	6123	2207	2111	2111
query14_1	1995	1988	1987	1987
query15	178	117	113	113
query16	929	375	314	314
query17	797	466	368	368
query18	2337	328	237	237
query19	169	152	116	116
query20	73	69	69	69
query21	207	119	102	102
query22	5426	5186	5145	5145
query23	6717	6282	5820	5820
query23_1	6134	6113	5900	5900
query24	7359	1083	770	770
query24_1	781	799	783	783
query25	438	344	234	234
query26	1255	261	165	165
query27	2714	449	277	277
query28	4655	1490	1499	1490
query29	927	435	350	350
query30	272	179	153	153
query31	845	423	346	346
query32	99	48	45	45
query33	464	221	170	170
query34	983	803	491	491
query35	399	394	345	345
query36	555	560	517	517
query37	119	83	69	69
query38	994	848	804	804
query39	486	493	465	465
query39_1	492	435	451	435
query40	216	117	108	108
query41	56	51	52	51
query42	87	81	80	80
query43	242	244	208	208
query44	1027	564	546	546
query45	114	104	102	102
query46	810	831	536	536
query47	756	767	713	713
query48	305	306	214	214
query49	530	233	187	187
query50	873	347	267	267
query51	7929	8001	7935	7935
query52	78	70	75	70
query53	210	229	167	167
query54	244	198	192	192
query55	77	63	54	54
query56	231	214	217	214
query57	801	675	655	655
query58	220	199	197	197
query59	1224	1230	1093	1093
query60	262	212	183	183
query61	129	126	112	112
query62	346	238	182	182
query63	178	155	155	155
query64	2847	818	705	705
query65	1577	1543	1609	1543
query66	1840	344	241	241
query67	9938	9492	9512	9492
query68	3032	1263	750	750
query69	376	245	200	200
query70	681	605	616	605
query71	286	263	257	257
query72	2359	1705	1600	1600
query73	650	630	362	362
query74	2000	1224	1139	1139
query75	1241	1159	1011	1011
query76	2396	772	560	560
query77	257	254	215	215
query78	3883	3769	3327	3327
query79	2780	824	596	596
query80	1617	401	347	347
query81	529	200	174	174
query82	682	126	103	103
query83	313	246	226	226
query84	307	119	99	99
query85	897	466	373	373
query86	478	177	171	171
query87	998	972	889	889
query88	3041	2136	2159	2136
query89	315	221	205	205
query90	2073	146	143	143
query91	151	142	125	125
query92	66	47	42	42
query93	2083	1224	731	731
query94	663	251	222	222
query95	617	421	339	339
query96	772	579	275	275
query97	1016	1037	987	987
query98	172	137	132	132
query99	413	349	316	316
Total cold run time: 180296 ms
Total hot run time: 82586 ms

@hello-stephen

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

query1	0.00	0.01	0.00
query2	0.07	0.04	0.04
query3	0.25	0.10	0.11
query4	1.60	0.11	0.09
query5	0.17	0.16	0.15
query6	1.25	0.69	0.71
query7	0.03	0.00	0.01
query8	0.05	0.03	0.03
query9	0.29	0.22	0.21
query10	0.35	0.36	0.34
query11	0.16	0.11	0.12
query12	0.15	0.12	0.12
query13	0.31	0.29	0.30
query14	0.45	0.46	0.44
query15	0.35	0.35	0.36
query16	0.24	0.22	0.22
query17	0.70	0.69	0.71
query18	0.19	0.18	0.17
query19	1.21	1.11	1.21
query20	0.01	0.01	0.01
query21	15.45	0.16	0.11
query22	5.08	0.04	0.04
query23	16.15	0.24	0.11
query24	2.99	0.31	0.26
query25	0.11	0.05	0.04
query26	0.78	0.16	0.12
query27	0.04	0.03	0.02
query28	3.71	0.59	0.27
query29	12.45	3.19	2.53
query30	0.26	0.11	0.13
query31	2.75	0.36	0.18
query32	3.52	0.30	0.23
query33	1.37	1.39	1.47
query34	15.40	2.14	1.75
query35	1.74	1.72	1.69
query36	0.46	0.29	0.28
query37	0.07	0.04	0.04
query38	0.05	0.03	0.03
query39	0.03	0.02	0.02
query40	0.12	0.07	0.08
query41	0.09	0.03	0.02
query42	0.03	0.02	0.02
query43	0.03	0.02	0.03
Total cold run time: 90.51 s
Total hot run time: 14.51 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 62.17% (28906/46492)
Line Coverage 47.23% (302466/640414)
Region Coverage 42.90% (244050/568866)
Branch Coverage 44.48% (113636/255460)

@YanzhiJin5

Copy link
Copy Markdown
Author

run external

@YanzhiJin5

Copy link
Copy Markdown
Author

Hi @yiguolei, could you please take a look when convenient?

All functional/build/regression TeamCity checks are now green. The only remaining failed TeamCity check is check_coverage, which has failed twice during CI setup before coverage processing.

Both runs failed because the CI runner could not resolve either LDB toolchain OSS hostname, on two different TeamCity agents. llvm-profdata / llvm-cov were never reached, so this does not appear to be a coverage-threshold failure.

The remaining GitHub workflows also require maintainer approval.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants