You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "csun5285 (via GitHub)" <gi...@apache.org> on 2026/05/25 02:14:44 UTC
[PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
csun5285 opened a new pull request, #63570:
URL: https://github.com/apache/doris/pull/63570
V3 layout: |data1..dataN|varuint_len1..varuint_lenN|data_block_size(u32)|num_elems(u32)|
Compared to V2 (length and data interleaved per entry), V3 lets the pre-decoder memcpy the entire binary payload in a single shot and walk the contiguous varuint length block once to fill the V1 offsets array, with no data-pointer-vs-length-pointer dependency between the two passes.
Benchmark (15-rep x 2s median, V3 / V2 speedup at 256 KiB page):
8B: 3.56x 16B: 3.07x 32B: 2.46x 64B: 2.63x
128B: 2.25x 256B: 1.39x 512B: 1.22x 1024B: 1.11x 4096B: 1.01x
V3 strictly does not lose to V2 across the tested grid.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592145438
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/167)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4601073332
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4602731660
# BE Regression && UT Coverage Report
Increment line coverage `85.45% (182/213)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.88% (28244/38227) |
| Line Coverage | 57.85% (307381/531360) |
| Region Coverage | 54.77% (258419/471834) |
| Branch Coverage | 56.17% (111861/199161) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592097743
# BE Regression && UT Coverage Report
Increment line coverage `85.45% (182/213)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.79% (28124/38112) |
| Line Coverage | 57.73% (305459/529125) |
| Region Coverage | 54.84% (255384/465672) |
| Branch Coverage | 56.35% (110310/195750) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531453939
<details>
<summary>TPC-DS: <b>Total hot run time: 172821 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 7f21734b8dcbbbf0b7d1992bda9c91c04ec17641, data reload: false
query5 4322 667 507 507
query6 346 218 205 205
query7 4205 566 307 307
query8 329 244 222 222
query9 8856 4058 4071 4058
query10 457 353 316 316
query11 5804 2802 2208 2208
query12 179 130 126 126
query13 1265 640 471 471
query14 6130 5520 5242 5242
query14_1 4586 4472 4482 4472
query15 210 209 189 189
query16 1033 468 420 420
query17 1146 723 593 593
query18 2718 488 354 354
query19 221 195 166 166
query20 139 131 128 128
query21 222 137 120 120
query22 13793 13690 13404 13404
query23 17326 16646 16310 16310
query23_1 16385 16332 16373 16332
query24 7459 1788 1307 1307
query24_1 1321 1323 1335 1323
query25 559 486 442 442
query26 1320 317 175 175
query27 2697 550 337 337
query28 4413 2002 2008 2002
query29 989 614 494 494
query30 308 246 198 198
query31 1128 1081 947 947
query32 89 83 74 74
query33 546 350 303 303
query34 1178 1125 654 654
query35 774 805 688 688
query36 1416 1438 1294 1294
query37 152 108 94 94
query38 3247 3175 3058 3058
query39 932 929 911 911
query39_1 922 883 901 883
query40 240 147 123 123
query41 65 67 62 62
query42 111 110 107 107
query43 326 336 295 295
query44
query45 213 206 207 206
query46 1108 1248 738 738
query47 2366 2417 2247 2247
query48 390 410 308 308
query49 630 493 404 404
query50 971 345 254 254
query51 4419 4365 4347 4347
query52 107 106 95 95
query53 250 281 215 215
query54 314 291 271 271
query55 95 94 86 86
query56 335 307 304 304
query57 1438 1418 1346 1346
query58 288 279 268 268
query59 1609 1679 1508 1508
query60 332 333 308 308
query61 159 152 156 152
query62 699 668 593 593
query63 245 206 210 206
query64 2399 805 638 638
query65
query66 1677 489 353 353
query67 30118 30133 29944 29944
query68
query69 493 351 304 304
query70 1104 1056 1005 1005
query71 316 275 270 270
query72 3050 2888 2604 2604
query73 864 775 429 429
query74 5128 4929 4809 4809
query75 2744 2662 2302 2302
query76 2283 1160 816 816
query77 419 422 343 343
query78 12453 12631 11951 11951
query79 1461 1034 763 763
query80 680 592 485 485
query81 457 291 250 250
query82 1347 165 130 130
query83 362 291 258 258
query84 266 149 120 120
query85 969 540 457 457
query86 401 353 366 353
query87 3443 3440 3255 3255
query88 3624 2855 2864 2855
query89 440 402 348 348
query90 1913 186 182 182
query91 180 175 141 141
query92 81 81 71 71
query93 1534 1474 843 843
query94 541 357 311 311
query95 681 399 340 340
query96 1102 831 351 351
query97 2749 2730 2608 2608
query98 246 237 223 223
query99 1186 1148 1039 1039
Total cold run time: 255331 ms
Total hot run time: 172821 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531454176
<details>
<summary>TPC-DS: <b>Total hot run time: 173713 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit c589ceabb09942f372b9c8ccd49d652b72dd9a71, data reload: false
query5 4308 653 522 522
query6 331 214 211 211
query7 4244 565 308 308
query8 334 237 232 232
query9 8819 4207 4163 4163
query10 464 357 303 303
query11 5804 2413 2187 2187
query12 192 128 129 128
query13 1285 630 436 436
query14 6135 5533 5233 5233
query14_1 4549 4572 4536 4536
query15 220 206 186 186
query16 1028 500 478 478
query17 1192 727 584 584
query18 2526 484 349 349
query19 220 196 157 157
query20 136 136 133 133
query21 219 139 119 119
query22 13645 13575 13421 13421
query23 17446 16643 16314 16314
query23_1 16435 16487 16540 16487
query24 7807 1791 1307 1307
query24_1 1348 1332 1322 1322
query25 541 486 425 425
query26 1329 326 172 172
query27 2704 552 350 350
query28 4442 1999 2035 1999
query29 988 641 498 498
query30 312 243 206 206
query31 1144 1085 952 952
query32 93 78 73 73
query33 543 366 301 301
query34 1220 1126 654 654
query35 798 796 704 704
query36 1398 1425 1264 1264
query37 164 108 95 95
query38 3216 3163 3092 3092
query39 930 938 939 938
query39_1 871 898 889 889
query40 247 144 122 122
query41 68 64 64 64
query42 110 111 114 111
query43 334 338 312 312
query44
query45 218 205 203 203
query46 1096 1164 759 759
query47 2383 2441 2278 2278
query48 405 426 296 296
query49 631 502 395 395
query50 1036 347 258 258
query51 4409 4402 4318 4318
query52 114 107 94 94
query53 249 278 205 205
query54 327 278 256 256
query55 92 90 88 88
query56 298 316 313 313
query57 1444 1424 1348 1348
query58 300 278 274 274
query59 1593 1734 1500 1500
query60 322 323 307 307
query61 166 162 164 162
query62 704 689 595 595
query63 251 208 208 208
query64 2450 803 651 651
query65
query66 1714 500 383 383
query67 30011 29993 29875 29875
query68
query69 476 363 323 323
query70 1061 995 1025 995
query71 310 283 279 279
query72 3291 2946 2525 2525
query73 845 785 443 443
query74 5084 5003 4793 4793
query75 2701 2635 2293 2293
query76 2278 1131 789 789
query77 402 410 341 341
query78 12460 12558 11870 11870
query79 1472 1047 772 772
query80 645 538 451 451
query81 452 291 240 240
query82 1392 157 123 123
query83 363 279 252 252
query84 255 150 109 109
query85 915 552 468 468
query86 419 360 328 328
query87 3427 3393 3279 3279
query88 3913 2812 2778 2778
query89 452 400 345 345
query90 1959 189 187 187
query91 182 169 153 153
query92 83 79 73 73
query93 1646 1435 870 870
query94 535 356 300 300
query95 683 487 360 360
query96 1119 796 351 351
query97 2779 2704 2633 2633
query98 236 230 227 227
query99 1180 1158 1039 1039
Total cold run time: 256413 ms
Total hot run time: 173713 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531733315
# BE UT Coverage Report
Increment line coverage `74.75% (148/198)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/7f21734b8dcbbbf0b7d1992bda9c91c04ec17641_7f21734b8dcbbbf0b7d1992bda9c91c04ec17641/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/7f21734b8dcbbbf0b7d1992bda9c91c04ec17641_7f21734b8dcbbbf0b7d1992bda9c91c04ec17641/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.79% (20864/38791) |
| Line Coverage | 37.37% (197699/529005) |
| Region Coverage | 33.68% (154894/459847) |
| Branch Coverage | 34.65% (67381/194440) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4604360121
<details>
<summary>TPC-H: <b>Total hot run time: 29030 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 13df7db0a5820578528956cb4070bf374d8b6a4b, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17797 4156 4077 4077
q2 q3 10886 1420 831 831
q4 4768 493 355 355
q5 8348 898 610 610
q6 336 204 143 143
q7 925 832 655 655
q8 10756 1619 1575 1575
q9 7165 4500 4450 4450
q10 6818 1851 1550 1550
q11 441 287 258 258
q12 672 433 295 295
q13 18233 3459 2777 2777
q14 266 259 255 255
q15 q16 833 785 715 715
q17 1053 896 1063 896
q18 6962 5762 5539 5539
q19 1183 1367 1080 1080
q20 528 429 266 266
q21 5686 2726 2404 2404
q22 457 355 299 299
Total cold run time: 104113 ms
Total hot run time: 29030 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4394 4291 4280 4280
q2 q3 4573 4947 4388 4388
q4 2106 2221 1415 1415
q5 4471 4338 4434 4338
q6 333 213 169 169
q7 2321 1952 1767 1767
q8 2562 2216 2193 2193
q9 8364 7918 7983 7918
q10 4847 4757 4276 4276
q11 581 455 420 420
q12 808 766 558 558
q13 3367 3667 3005 3005
q14 298 314 272 272
q15 q16 726 771 651 651
q17 1386 1479 1362 1362
q18 7710 7363 7287 7287
q19 1173 1149 1106 1106
q20 2251 2230 1953 1953
q21 5316 4701 4535 4535
q22 527 451 425 425
Total cold run time: 58114 ms
Total hot run time: 52318 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531132765
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4591080399
# FE UT Coverage Report
Increment line coverage `` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_08586fee75495cede618a527246dbcf94612c1a0/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_08586fee75495cede618a527246dbcf94612c1a0/fe_report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592288566
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/167)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4590863815
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/534d64545df37e0fc80d787e03fd2fbc6f701316_534d64545df37e0fc80d787e03fd2fbc6f701316_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/534d64545df37e0fc80d787e03fd2fbc6f701316_534d64545df37e0fc80d787e03fd2fbc6f701316_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 78.34% (1906/2433) |
| Line Coverage | 64.76% (33979/52468) |
| Region Coverage | 65.28% (17517/26833) |
| Branch Coverage | 53.97% (9299/17230) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592134748
# BE Regression && UT Coverage Report
Increment line coverage `85.45% (182/213)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.80% (28126/38112) |
| Line Coverage | 57.74% (305526/529125) |
| Region Coverage | 54.86% (255457/465672) |
| Branch Coverage | 56.37% (110346/195750) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on code in PR #63570:
URL: https://github.com/apache/doris/pull/63570#discussion_r3349517842
##########
be/src/storage/segment/binary_plain_page_v3_pre_decoder.h:
##########
@@ -0,0 +1,190 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <cstring>
+#include <vector>
+
+#include "storage/cache/page_cache.h"
+#include "storage/segment/binary_plain_page_v2_pre_decoder.h" // BinaryPlainV1Entry, write_binary_plain_v1_output
+#include "storage/segment/encoding_info.h"
+#include "util/coding.h"
+
+namespace doris {
+namespace segment_v2 {
+
+/**
+ * @brief Pre-decoder for BinaryPlainPageV3.
+ *
+ * Converts the V3 layout (contiguous data + contiguous varuint length block)
+ * to the V1 layout (offsets array trailer) so the cached page can be served
+ * with O(1) seek.
+ *
+ * V3 format (input):
+ * |binary1|binary2|...|binaryN|varuint_len1|...|varuint_lenN|
+ * |data_block_size(32-bit)|num_elems(32-bit)|
+ *
+ * V1 format (output, same as BinaryPlainPageV2PreDecoder):
+ * |binary1|...|binaryN|offset1(32-bit)|...|offsetN(32-bit)|num_elems(32-bit)|
+ *
+ * The builder stores the same bytes as V1/V2 (CHAR keeps its '\0' padding, VARCHAR
+ * does not); only the layout differs. IS_CHAR selects how the stored lengths are
+ * treated on read, mirroring V2:
+ * - IS_CHAR=false: values carry no padding, so the binary block is memcpy'd in a
+ * single shot and the varint loop only fills the running offsets array — the two
+ * passes are independent and the data is touched exactly once. This is the win vs
+ * V2 (which chases a length pointer per entry). Used for all non-CHAR binary types
+ * (VARCHAR/STRING/JSONB/VARIANT/HLL/BITMAP/QUANTILE_STATE/AGG_STATE).
+ * - IS_CHAR=true: each entry is strnlen'd to drop the trailing '\0' padding that CHAR
+ * values carry on disk, then the V1 page is built from the logical lengths. Selected
+ * for (CHAR, PLAIN_ENCODING_V3) on read (e.g. the CHAR dictionary word page).
+ */
+template <bool IS_CHAR>
+struct BinaryPlainPageV3PreDecoder : public DataPagePreDecoder {
+ Status decode(std::unique_ptr<DataPage>* page, Slice* page_slice, size_t size_of_tail,
+ bool _use_cache, segment_v2::PageTypePB page_type, const std::string& file_path,
+ size_t size_of_prefix = 0) override {
+ // V3 trailer is two u32 words: data_block_size then num_elems.
+ constexpr size_t kV3TrailerSize = 2 * sizeof(uint32_t);
+
+ if (page_slice->size < kV3TrailerSize + size_of_tail) {
+ return Status::Corruption("Invalid V3 page size: {}, expected at least {} in file: {}",
+ page_slice->size, kV3TrailerSize + size_of_tail, file_path);
+ }
+
+ // page_slice->size >= kV3TrailerSize + size_of_tail is enforced above,
+ // so data.size = page_slice->size - size_of_tail >= kV3TrailerSize.
+ Slice data(page_slice->data, page_slice->size - size_of_tail);
+
+ const uint8_t* data_begin = reinterpret_cast<const uint8_t*>(data.data);
+ const uint8_t* trailer_ptr = data_begin + data.size - kV3TrailerSize;
+ uint32_t data_block_size = decode_fixed32_le(trailer_ptr);
+ uint32_t num_elems = decode_fixed32_le(trailer_ptr + sizeof(uint32_t));
+
+ // Use subtraction form to avoid uint32_t wraparound on a malicious
+ // data_block_size close to UINT32_MAX. data.size >= kV3TrailerSize.
+ if (data_block_size > data.size - kV3TrailerSize) {
+ return Status::Corruption("V3 data_block_size {} exceeds available data {} in file: {}",
+ data_block_size, data.size - kV3TrailerSize, file_path);
+ }
+
+ const uint8_t* lengths_ptr = data_begin + data_block_size;
+ const uint8_t* lengths_limit = trailer_ptr;
+
+ if constexpr (IS_CHAR) {
+ // ---- CHAR path: strnlen each entry to strip trailing '\0' padding. ----
+ // Walk the contiguous data block in lockstep with the length block:
+ // entry i starts at data_begin + running_raw and is `len` bytes wide.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running_raw = 0;
+ std::vector<BinaryPlainV1Entry> entries;
+ entries.reserve(num_elems);
+ uint32_t total_out_len = 0;
+ for (uint32_t i = 0; i < num_elems; ++i) {
+ if (ptr >= lengths_limit) {
+ return Status::Corruption(
+ "V3 unexpected end of length block at element {} in file: {}", i,
+ file_path);
+ }
+ uint32_t len = 0;
+ ptr = decode_varint32_ptr(ptr, lengths_limit, &len);
+ if (ptr == nullptr) {
+ return Status::Corruption(
+ "V3 failed to decode varuint at element {} in file: {}", i, file_path);
+ }
+ if (len > data_block_size - running_raw) {
+ return Status::Corruption(
+ "V3 entry {} length {} overflows data block in file: {}", i, len,
+ file_path);
+ }
+ const uint8_t* entry_data = data_begin + running_raw;
+ uint32_t out_len = static_cast<uint32_t>(
+ strnlen(reinterpret_cast<const char*>(entry_data), len));
+ entries.push_back({entry_data, out_len});
+ total_out_len += out_len;
+ running_raw += len;
+ }
+
+ if (running_raw != data_block_size) {
+ return Status::Corruption("V3 sum of lengths {} != data_block_size {} in file: {}",
+ running_raw, data_block_size, file_path);
+ }
+
+ return write_binary_plain_v1_output(entries, num_elems, total_out_len, *page_slice,
+ size_of_tail, size_of_prefix, _use_cache, page_type,
+ page, page_slice);
+ } else {
+ // ---- Fast path (non-CHAR): lengths are already logical. ----
+ const size_t offsets_size = static_cast<size_t>(num_elems) * sizeof(uint32_t);
+ const size_t v1_data_size = data_block_size + offsets_size + sizeof(uint32_t);
+ const size_t total_size = size_of_prefix + v1_data_size + size_of_tail;
+
+ std::unique_ptr<DataPage> decoded_page =
+ std::make_unique<DataPage>(total_size, _use_cache, page_type);
+ Slice decoded_slice(decoded_page->data(), total_size);
+ char* output = decoded_slice.data + size_of_prefix;
+
+ // 1. Single memcpy of the entire binary payload.
+ if (data_block_size > 0) {
+ memcpy(output, data_begin, data_block_size);
+ }
+ output += data_block_size;
+
+ // 2. Walk varints once, write the running offsets array.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running = 0;
+ for (uint32_t i = 0; i < num_elems; ++i) {
+ if (ptr >= lengths_limit) {
+ return Status::Corruption(
+ "V3 unexpected end of length block at element {} in file: {}", i,
+ file_path);
+ }
+ uint32_t len = 0;
+ ptr = decode_varint32_ptr(ptr, lengths_limit, &len);
+ if (ptr == nullptr) {
+ return Status::Corruption(
+ "V3 failed to decode varuint at element {} in file: {}", i, file_path);
+ }
+ encode_fixed32_le(reinterpret_cast<uint8_t*>(output), running);
+ output += sizeof(uint32_t);
+ running += len;
Review Comment:
no block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531407419
<details>
<summary>TPC-H: <b>Total hot run time: 31963 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit c589ceabb09942f372b9c8ccd49d652b72dd9a71, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17593 4049 3997 3997
q2 q3 10832 1465 801 801
q4 4758 488 349 349
q5 9819 2345 2124 2124
q6 392 176 140 140
q7 946 790 645 645
q8 9585 1870 1627 1627
q9 7117 4995 5005 4995
q10 6467 2251 1917 1917
q11 450 283 253 253
q12 700 442 308 308
q13 18190 3470 2857 2857
q14 266 265 246 246
q15 q16 832 791 730 730
q17 942 893 957 893
q18 7076 5835 6304 5835
q19 1203 1295 1118 1118
q20 531 431 286 286
q21 5916 2810 2514 2514
q22 460 373 328 328
Total cold run time: 104075 ms
Total hot run time: 31963 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4790 4813 4803 4803
q2 q3 4998 5431 4619 4619
q4 2181 2221 1438 1438
q5 4817 4832 4752 4752
q6 239 181 135 135
q7 1879 1754 1580 1580
q8 2505 2143 1963 1963
q9 7474 7456 7505 7456
q10 4814 4690 4247 4247
q11 548 393 363 363
q12 749 755 539 539
q13 3065 3359 2767 2767
q14 269 277 268 268
q15 q16 688 698 618 618
q17 1305 1325 1287 1287
q18 7436 6776 6861 6776
q19 1087 1097 1109 1097
q20 2253 2242 1940 1940
q21 5441 4660 4545 4545
q22 525 481 406 406
Total cold run time: 57063 ms
Total hot run time: 51599 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531017182
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4611725372
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4601705858
# FE UT Coverage Report
Increment line coverage `` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b/fe_report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4590627463
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4602579606
# BE UT Coverage Report
Increment line coverage `80.28% (171/213)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/13df7db0a5820578528956cb4070bf374d8b6a4b_13df7db0a5820578528956cb4070bf374d8b6a4b/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/13df7db0a5820578528956cb4070bf374d8b6a4b_13df7db0a5820578528956cb4070bf374d8b6a4b/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 54.13% (21131/39037) |
| Line Coverage | 37.68% (200744/532736) |
| Region Coverage | 33.80% (157965/467413) |
| Branch Coverage | 34.77% (68981/198415) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4611724853
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon merged PR #63570:
URL: https://github.com/apache/doris/pull/63570
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #63570:
URL: https://github.com/apache/doris/pull/63570#discussion_r3333504454
##########
be/src/storage/segment/binary_plain_page_v3_pre_decoder.h:
##########
@@ -0,0 +1,190 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <cstring>
+#include <vector>
+
+#include "storage/cache/page_cache.h"
+#include "storage/segment/binary_plain_page_v2_pre_decoder.h" // BinaryPlainV1Entry, write_binary_plain_v1_output
+#include "storage/segment/encoding_info.h"
+#include "util/coding.h"
+
+namespace doris {
+namespace segment_v2 {
+
+/**
+ * @brief Pre-decoder for BinaryPlainPageV3.
+ *
+ * Converts the V3 layout (contiguous data + contiguous varuint length block)
+ * to the V1 layout (offsets array trailer) so the cached page can be served
+ * with O(1) seek.
+ *
+ * V3 format (input):
+ * |binary1|binary2|...|binaryN|varuint_len1|...|varuint_lenN|
+ * |data_block_size(32-bit)|num_elems(32-bit)|
+ *
+ * V1 format (output, same as BinaryPlainPageV2PreDecoder):
+ * |binary1|...|binaryN|offset1(32-bit)|...|offsetN(32-bit)|num_elems(32-bit)|
+ *
+ * The builder stores the same bytes as V1/V2 (CHAR keeps its '\0' padding, VARCHAR
+ * does not); only the layout differs. IS_CHAR selects how the stored lengths are
+ * treated on read, mirroring V2:
+ * - IS_CHAR=false: values carry no padding, so the binary block is memcpy'd in a
+ * single shot and the varint loop only fills the running offsets array — the two
+ * passes are independent and the data is touched exactly once. This is the win vs
+ * V2 (which chases a length pointer per entry). Used for all non-CHAR binary types
+ * (VARCHAR/STRING/JSONB/VARIANT/HLL/BITMAP/QUANTILE_STATE/AGG_STATE).
+ * - IS_CHAR=true: each entry is strnlen'd to drop the trailing '\0' padding that CHAR
+ * values carry on disk, then the V1 page is built from the logical lengths. Selected
+ * for (CHAR, PLAIN_ENCODING_V3) on read (e.g. the CHAR dictionary word page).
+ */
+template <bool IS_CHAR>
+struct BinaryPlainPageV3PreDecoder : public DataPagePreDecoder {
+ Status decode(std::unique_ptr<DataPage>* page, Slice* page_slice, size_t size_of_tail,
+ bool _use_cache, segment_v2::PageTypePB page_type, const std::string& file_path,
+ size_t size_of_prefix = 0) override {
+ // V3 trailer is two u32 words: data_block_size then num_elems.
+ constexpr size_t kV3TrailerSize = 2 * sizeof(uint32_t);
+
+ if (page_slice->size < kV3TrailerSize + size_of_tail) {
+ return Status::Corruption("Invalid V3 page size: {}, expected at least {} in file: {}",
+ page_slice->size, kV3TrailerSize + size_of_tail, file_path);
+ }
+
+ // page_slice->size >= kV3TrailerSize + size_of_tail is enforced above,
+ // so data.size = page_slice->size - size_of_tail >= kV3TrailerSize.
+ Slice data(page_slice->data, page_slice->size - size_of_tail);
+
+ const uint8_t* data_begin = reinterpret_cast<const uint8_t*>(data.data);
+ const uint8_t* trailer_ptr = data_begin + data.size - kV3TrailerSize;
+ uint32_t data_block_size = decode_fixed32_le(trailer_ptr);
+ uint32_t num_elems = decode_fixed32_le(trailer_ptr + sizeof(uint32_t));
+
+ // Use subtraction form to avoid uint32_t wraparound on a malicious
+ // data_block_size close to UINT32_MAX. data.size >= kV3TrailerSize.
+ if (data_block_size > data.size - kV3TrailerSize) {
+ return Status::Corruption("V3 data_block_size {} exceeds available data {} in file: {}",
+ data_block_size, data.size - kV3TrailerSize, file_path);
+ }
+
+ const uint8_t* lengths_ptr = data_begin + data_block_size;
+ const uint8_t* lengths_limit = trailer_ptr;
+
+ if constexpr (IS_CHAR) {
+ // ---- CHAR path: strnlen each entry to strip trailing '\0' padding. ----
+ // Walk the contiguous data block in lockstep with the length block:
+ // entry i starts at data_begin + running_raw and is `len` bytes wide.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running_raw = 0;
+ std::vector<BinaryPlainV1Entry> entries;
+ entries.reserve(num_elems);
+ uint32_t total_out_len = 0;
+ for (uint32_t i = 0; i < num_elems; ++i) {
+ if (ptr >= lengths_limit) {
+ return Status::Corruption(
+ "V3 unexpected end of length block at element {} in file: {}", i,
+ file_path);
+ }
+ uint32_t len = 0;
+ ptr = decode_varint32_ptr(ptr, lengths_limit, &len);
+ if (ptr == nullptr) {
+ return Status::Corruption(
+ "V3 failed to decode varuint at element {} in file: {}", i, file_path);
+ }
+ if (len > data_block_size - running_raw) {
+ return Status::Corruption(
+ "V3 entry {} length {} overflows data block in file: {}", i, len,
+ file_path);
+ }
+ const uint8_t* entry_data = data_begin + running_raw;
+ uint32_t out_len = static_cast<uint32_t>(
+ strnlen(reinterpret_cast<const char*>(entry_data), len));
+ entries.push_back({entry_data, out_len});
+ total_out_len += out_len;
+ running_raw += len;
+ }
+
+ if (running_raw != data_block_size) {
+ return Status::Corruption("V3 sum of lengths {} != data_block_size {} in file: {}",
+ running_raw, data_block_size, file_path);
+ }
+
+ return write_binary_plain_v1_output(entries, num_elems, total_out_len, *page_slice,
+ size_of_tail, size_of_prefix, _use_cache, page_type,
+ page, page_slice);
+ } else {
+ // ---- Fast path (non-CHAR): lengths are already logical. ----
+ const size_t offsets_size = static_cast<size_t>(num_elems) * sizeof(uint32_t);
+ const size_t v1_data_size = data_block_size + offsets_size + sizeof(uint32_t);
+ const size_t total_size = size_of_prefix + v1_data_size + size_of_tail;
+
+ std::unique_ptr<DataPage> decoded_page =
+ std::make_unique<DataPage>(total_size, _use_cache, page_type);
+ Slice decoded_slice(decoded_page->data(), total_size);
+ char* output = decoded_slice.data + size_of_prefix;
+
+ // 1. Single memcpy of the entire binary payload.
+ if (data_block_size > 0) {
+ memcpy(output, data_begin, data_block_size);
+ }
+ output += data_block_size;
+
+ // 2. Walk varints once, write the running offsets array.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running = 0;
Review Comment:
我们这个offset 存储，对于第0个元素，存储的是啥？
比如这个循环，假如num_elems == 1， 此时感觉output 里只有一个0，那么读取的时候，我们怎么知道这个string 多长呢？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4593160297
<details>
<summary>TPC-DS: <b>Total hot run time: 170846 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 534d64545df37e0fc80d787e03fd2fbc6f701316, data reload: false
query5 4347 664 511 511
query6 356 222 202 202
query7 4228 586 306 306
query8 341 242 223 223
query9 8819 4156 4031 4031
query10 460 374 296 296
query11 5766 2354 2172 2172
query12 188 131 126 126
query13 1285 633 432 432
query14 6160 5432 5120 5120
query14_1 4443 4456 4397 4397
query15 214 201 183 183
query16 1012 456 444 444
query17 1146 742 588 588
query18 2490 510 355 355
query19 218 203 173 173
query20 140 134 138 134
query21 219 139 114 114
query22 13645 13568 13349 13349
query23 17427 16601 16218 16218
query23_1 16451 16387 16362 16362
query24 7575 1754 1313 1313
query24_1 1318 1310 1303 1303
query25 548 491 419 419
query26 1312 306 167 167
query27 2723 553 349 349
query28 4444 2054 2013 2013
query29 1001 631 506 506
query30 306 237 198 198
query31 1145 1066 950 950
query32 91 76 72 72
query33 552 358 304 304
query34 1224 1151 654 654
query35 797 822 715 715
query36 1424 1469 1293 1293
query37 163 111 97 97
query38 3223 3130 3061 3061
query39 936 930 887 887
query39_1 890 879 886 879
query40 244 156 133 133
query41 73 69 68 68
query42 119 118 114 114
query43 335 339 297 297
query44
query45 218 212 205 205
query46 1117 1221 739 739
query47 2382 2447 2264 2264
query48 386 410 315 315
query49 672 516 413 413
query50 1036 357 262 262
query51 4393 4405 4203 4203
query52 110 112 99 99
query53 259 288 210 210
query54 341 292 278 278
query55 96 95 91 91
query56 318 324 334 324
query57 1453 1439 1338 1338
query58 317 291 285 285
query59 1623 1716 1475 1475
query60 344 343 335 335
query61 187 182 179 179
query62 713 669 598 598
query63 268 207 222 207
query64 2484 848 697 697
query65
query66 1793 491 384 384
query67 29745 29613 29650 29613
query68
query69 475 343 312 312
query70 1113 1038 1019 1019
query71 315 291 270 270
query72 3104 2706 2415 2415
query73 843 752 435 435
query74 5133 4950 4773 4773
query75 2726 2613 2254 2254
query76 2268 1133 778 778
query77 416 417 340 340
query78 12500 12476 11932 11932
query79 1464 1039 780 780
query80 656 542 462 462
query81 462 281 252 252
query82 1359 153 124 124
query83 363 282 250 250
query84 260 141 114 114
query85 900 547 471 471
query86 405 345 350 345
query87 3419 3368 3237 3237
query88 3658 2762 2746 2746
query89 453 398 343 343
query90 1985 181 180 180
query91 187 169 136 136
query92 82 81 74 74
query93 1523 1450 862 862
query94 583 334 329 329
query95 681 471 359 359
query96 1084 830 343 343
query97 2755 2711 2648 2648
query98 239 227 230 227
query99 1161 1176 1041 1041
Total cold run time: 255673 ms
Total hot run time: 170846 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4611867052
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4604451656
<details>
<summary>TPC-DS: <b>Total hot run time: 169673 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 13df7db0a5820578528956cb4070bf374d8b6a4b, data reload: false
query5 4358 637 503 503
query6 459 203 181 181
query7 4820 568 308 308
query8 377 231 230 230
query9 8777 4127 4152 4127
query10 452 317 275 275
query11 5883 2433 2189 2189
query12 155 108 104 104
query13 1258 654 441 441
query14 6624 5483 5126 5126
query14_1 4462 4476 4498 4476
query15 210 201 185 185
query16 1062 464 471 464
query17 1132 743 617 617
query18 2733 489 359 359
query19 210 189 157 157
query20 116 114 109 109
query21 215 145 120 120
query22 13595 13647 13477 13477
query23 17537 16479 16185 16185
query23_1 16425 16390 16385 16385
query24 7583 1757 1308 1308
query24_1 1314 1342 1313 1313
query25 620 452 377 377
query26 1307 332 174 174
query27 2571 530 337 337
query28 4364 2044 2022 2022
query29 1102 611 494 494
query30 313 233 195 195
query31 1110 1084 961 961
query32 114 63 58 58
query33 522 320 261 261
query34 1172 1159 675 675
query35 778 815 680 680
query36 1374 1373 1197 1197
query37 153 109 94 94
query38 3220 3159 3058 3058
query39 930 929 895 895
query39_1 878 870 893 870
query40 217 124 102 102
query41 67 69 64 64
query42 99 97 94 94
query43 332 331 302 302
query44
query45 201 194 186 186
query46 1112 1191 711 711
query47 2328 2399 2228 2228
query48 384 440 299 299
query49 634 471 365 365
query50 971 339 262 262
query51 4414 4371 4259 4259
query52 90 91 79 79
query53 263 266 189 189
query54 271 250 214 214
query55 80 76 74 74
query56 248 231 226 226
query57 1414 1377 1314 1314
query58 243 213 222 213
query59 1582 1690 1445 1445
query60 286 255 229 229
query61 170 156 160 156
query62 700 655 569 569
query63 236 192 185 185
query64 2529 794 673 673
query65
query66 1727 478 345 345
query67 29858 29183 29537 29183
query68
query69 428 311 265 265
query70 989 957 975 957
query71 307 216 203 203
query72 2975 2736 2396 2396
query73 871 780 443 443
query74 5150 4977 4792 4792
query75 2647 2591 2247 2247
query76 2324 1153 792 792
query77 370 386 297 297
query78 12404 12451 11925 11925
query79 1302 997 799 799
query80 561 496 429 429
query81 453 281 250 250
query82 317 160 126 126
query83 358 311 253 253
query84 277 140 120 120
query85 879 536 447 447
query86 367 300 306 300
query87 3381 3356 3192 3192
query88 3673 2822 2766 2766
query89 432 378 338 338
query90 1972 201 197 197
query91 179 169 143 143
query92 67 60 57 57
query93 1508 1524 951 951
query94 542 358 317 317
query95 678 493 363 363
query96 1054 858 351 351
query97 2685 2716 2558 2558
query98 230 208 209 208
query99 1151 1173 1034 1034
Total cold run time: 251578 ms
Total hot run time: 169673 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on code in PR #63570:
URL: https://github.com/apache/doris/pull/63570#discussion_r3349520287
##########
be/src/storage/segment/encoding_info.cpp:
##########
@@ -381,10 +406,10 @@ EncodingInfoResolver::EncodingInfoResolver() {
_set_v3_default<FieldType::OLAP_FIELD_TYPE_DECIMAL256, BIT_SHUFFLE>();
_set_v3_default<FieldType::OLAP_FIELD_TYPE_IPV4, BIT_SHUFFLE>();
_set_v3_default<FieldType::OLAP_FIELD_TYPE_IPV6, BIT_SHUFFLE>();
- _set_v3_default<FieldType::OLAP_FIELD_TYPE_HLL, PLAIN_ENCODING_V2>();
Review Comment:
no rollback
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4602836820
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/344)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_13df7db0a5820578528956cb4070bf374d8b6a4b_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592172195
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/167)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4592201901
# BE Regression && UT Coverage Report
Increment line coverage `85.45% (182/213)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63570_534d64545df37e0fc80d787e03fd2fbc6f701316_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.83% (28150/38127) |
| Line Coverage | 57.80% (305883/529166) |
| Region Coverage | 54.90% (255677/465687) |
| Branch Coverage | 56.46% (110516/195750) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4593074508
<details>
<summary>TPC-H: <b>Total hot run time: 29310 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 534d64545df37e0fc80d787e03fd2fbc6f701316, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17765 4060 4022 4022
q2 q3 10782 1490 822 822
q4 4680 480 350 350
q5 7556 884 582 582
q6 185 169 137 137
q7 782 853 641 641
q8 9335 1618 1613 1613
q9 5847 4481 4570 4481
q10 6768 1816 1551 1551
q11 438 274 260 260
q12 637 440 293 293
q13 18201 3398 2827 2827
q14 265 254 238 238
q15 q16 797 774 711 711
q17 1002 970 977 970
q18 7403 5734 5473 5473
q19 1173 1301 1140 1140
q20 512 410 265 265
q21 5965 2903 2604 2604
q22 467 393 330 330
Total cold run time: 100560 ms
Total hot run time: 29310 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 5177 4715 4800 4715
q2 q3 4975 5382 4711 4711
q4 2134 2227 1401 1401
q5 4822 4903 4795 4795
q6 234 170 129 129
q7 1854 1764 1580 1580
q8 2461 2122 2091 2091
q9 8018 7810 7452 7452
q10 4790 4685 4193 4193
q11 537 424 351 351
q12 715 747 525 525
q13 3011 3323 2712 2712
q14 263 286 254 254
q15 q16 686 696 611 611
q17 1282 1260 1268 1260
q18 7291 6819 6860 6819
q19 1109 1129 1135 1129
q20 2222 2222 1958 1958
q21 5304 4666 4528 4528
q22 532 458 412 412
Total cold run time: 57417 ms
Total hot run time: 51626 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) Add PLAIN_ENCODING_V3 binary plain page for the V3 storage format [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63570:
URL: https://github.com/apache/doris/pull/63570#discussion_r3349386415
##########
be/src/storage/segment/binary_plain_page_v3_pre_decoder.h:
##########
@@ -0,0 +1,190 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <cstring>
+#include <vector>
+
+#include "storage/cache/page_cache.h"
+#include "storage/segment/binary_plain_page_v2_pre_decoder.h" // BinaryPlainV1Entry, write_binary_plain_v1_output
+#include "storage/segment/encoding_info.h"
+#include "util/coding.h"
+
+namespace doris {
+namespace segment_v2 {
+
+/**
+ * @brief Pre-decoder for BinaryPlainPageV3.
+ *
+ * Converts the V3 layout (contiguous data + contiguous varuint length block)
+ * to the V1 layout (offsets array trailer) so the cached page can be served
+ * with O(1) seek.
+ *
+ * V3 format (input):
+ * |binary1|binary2|...|binaryN|varuint_len1|...|varuint_lenN|
+ * |data_block_size(32-bit)|num_elems(32-bit)|
+ *
+ * V1 format (output, same as BinaryPlainPageV2PreDecoder):
+ * |binary1|...|binaryN|offset1(32-bit)|...|offsetN(32-bit)|num_elems(32-bit)|
+ *
+ * The builder stores the same bytes as V1/V2 (CHAR keeps its '\0' padding, VARCHAR
+ * does not); only the layout differs. IS_CHAR selects how the stored lengths are
+ * treated on read, mirroring V2:
+ * - IS_CHAR=false: values carry no padding, so the binary block is memcpy'd in a
+ * single shot and the varint loop only fills the running offsets array — the two
+ * passes are independent and the data is touched exactly once. This is the win vs
+ * V2 (which chases a length pointer per entry). Used for all non-CHAR binary types
+ * (VARCHAR/STRING/JSONB/VARIANT/HLL/BITMAP/QUANTILE_STATE/AGG_STATE).
+ * - IS_CHAR=true: each entry is strnlen'd to drop the trailing '\0' padding that CHAR
+ * values carry on disk, then the V1 page is built from the logical lengths. Selected
+ * for (CHAR, PLAIN_ENCODING_V3) on read (e.g. the CHAR dictionary word page).
+ */
+template <bool IS_CHAR>
+struct BinaryPlainPageV3PreDecoder : public DataPagePreDecoder {
+ Status decode(std::unique_ptr<DataPage>* page, Slice* page_slice, size_t size_of_tail,
+ bool _use_cache, segment_v2::PageTypePB page_type, const std::string& file_path,
+ size_t size_of_prefix = 0) override {
+ // V3 trailer is two u32 words: data_block_size then num_elems.
+ constexpr size_t kV3TrailerSize = 2 * sizeof(uint32_t);
+
+ if (page_slice->size < kV3TrailerSize + size_of_tail) {
+ return Status::Corruption("Invalid V3 page size: {}, expected at least {} in file: {}",
+ page_slice->size, kV3TrailerSize + size_of_tail, file_path);
+ }
+
+ // page_slice->size >= kV3TrailerSize + size_of_tail is enforced above,
+ // so data.size = page_slice->size - size_of_tail >= kV3TrailerSize.
+ Slice data(page_slice->data, page_slice->size - size_of_tail);
+
+ const uint8_t* data_begin = reinterpret_cast<const uint8_t*>(data.data);
+ const uint8_t* trailer_ptr = data_begin + data.size - kV3TrailerSize;
+ uint32_t data_block_size = decode_fixed32_le(trailer_ptr);
+ uint32_t num_elems = decode_fixed32_le(trailer_ptr + sizeof(uint32_t));
+
+ // Use subtraction form to avoid uint32_t wraparound on a malicious
+ // data_block_size close to UINT32_MAX. data.size >= kV3TrailerSize.
+ if (data_block_size > data.size - kV3TrailerSize) {
+ return Status::Corruption("V3 data_block_size {} exceeds available data {} in file: {}",
+ data_block_size, data.size - kV3TrailerSize, file_path);
+ }
+
+ const uint8_t* lengths_ptr = data_begin + data_block_size;
+ const uint8_t* lengths_limit = trailer_ptr;
+
+ if constexpr (IS_CHAR) {
+ // ---- CHAR path: strnlen each entry to strip trailing '\0' padding. ----
+ // Walk the contiguous data block in lockstep with the length block:
+ // entry i starts at data_begin + running_raw and is `len` bytes wide.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running_raw = 0;
+ std::vector<BinaryPlainV1Entry> entries;
+ entries.reserve(num_elems);
+ uint32_t total_out_len = 0;
+ for (uint32_t i = 0; i < num_elems; ++i) {
+ if (ptr >= lengths_limit) {
+ return Status::Corruption(
+ "V3 unexpected end of length block at element {} in file: {}", i,
+ file_path);
+ }
+ uint32_t len = 0;
+ ptr = decode_varint32_ptr(ptr, lengths_limit, &len);
+ if (ptr == nullptr) {
+ return Status::Corruption(
+ "V3 failed to decode varuint at element {} in file: {}", i, file_path);
+ }
+ if (len > data_block_size - running_raw) {
+ return Status::Corruption(
+ "V3 entry {} length {} overflows data block in file: {}", i, len,
+ file_path);
+ }
+ const uint8_t* entry_data = data_begin + running_raw;
+ uint32_t out_len = static_cast<uint32_t>(
+ strnlen(reinterpret_cast<const char*>(entry_data), len));
+ entries.push_back({entry_data, out_len});
+ total_out_len += out_len;
+ running_raw += len;
+ }
+
+ if (running_raw != data_block_size) {
+ return Status::Corruption("V3 sum of lengths {} != data_block_size {} in file: {}",
+ running_raw, data_block_size, file_path);
+ }
+
+ return write_binary_plain_v1_output(entries, num_elems, total_out_len, *page_slice,
+ size_of_tail, size_of_prefix, _use_cache, page_type,
+ page, page_slice);
+ } else {
+ // ---- Fast path (non-CHAR): lengths are already logical. ----
+ const size_t offsets_size = static_cast<size_t>(num_elems) * sizeof(uint32_t);
+ const size_t v1_data_size = data_block_size + offsets_size + sizeof(uint32_t);
+ const size_t total_size = size_of_prefix + v1_data_size + size_of_tail;
+
+ std::unique_ptr<DataPage> decoded_page =
+ std::make_unique<DataPage>(total_size, _use_cache, page_type);
+ Slice decoded_slice(decoded_page->data(), total_size);
+ char* output = decoded_slice.data + size_of_prefix;
+
+ // 1. Single memcpy of the entire binary payload.
+ if (data_block_size > 0) {
+ memcpy(output, data_begin, data_block_size);
+ }
+ output += data_block_size;
+
+ // 2. Walk varints once, write the running offsets array.
+ const uint8_t* ptr = lengths_ptr;
+ uint32_t running = 0;
+ for (uint32_t i = 0; i < num_elems; ++i) {
+ if (ptr >= lengths_limit) {
+ return Status::Corruption(
+ "V3 unexpected end of length block at element {} in file: {}", i,
+ file_path);
+ }
+ uint32_t len = 0;
+ ptr = decode_varint32_ptr(ptr, lengths_limit, &len);
+ if (ptr == nullptr) {
+ return Status::Corruption(
+ "V3 failed to decode varuint at element {} in file: {}", i, file_path);
+ }
+ encode_fixed32_le(reinterpret_cast<uint8_t*>(output), running);
+ output += sizeof(uint32_t);
+ running += len;
Review Comment:
This non-CHAR path needs the same per-entry bounds check as the CHAR path before adding `len`. As written, a corrupt page can encode lengths whose 32-bit sum wraps back to `data_block_size`, so the final `running != data_block_size` check passes while the emitted V1 offsets point outside the copied data block. For example, with `data_block_size = 1`, `num_elems = 2`, and varint lengths `UINT32_MAX` and `2`, the first offset is `0`, the second emitted offset becomes `UINT32_MAX`, `running` wraps to `1`, and the page is accepted. The downstream `BinaryPlainPageDecoder` does not validate monotonic/in-range offsets, so later reads can use those invalid offsets. Please reject `len > data_block_size - running` before writing the offset/incrementing, and add a corruption test for this wraparound case.
##########
be/src/storage/segment/encoding_info.cpp:
##########
@@ -381,10 +406,10 @@ EncodingInfoResolver::EncodingInfoResolver() {
_set_v3_default<FieldType::OLAP_FIELD_TYPE_DECIMAL256, BIT_SHUFFLE>();
_set_v3_default<FieldType::OLAP_FIELD_TYPE_IPV4, BIT_SHUFFLE>();
_set_v3_default<FieldType::OLAP_FIELD_TYPE_IPV6, BIT_SHUFFLE>();
- _set_v3_default<FieldType::OLAP_FIELD_TYPE_HLL, PLAIN_ENCODING_V2>();
Review Comment:
This changes the persisted encoding written for V3 segments immediately, but I do not see a mixed-version or rollback gate. An older BE only knows encodings up to `PLAIN_ENCODING_V2` and its dict pre-decoder rejects unknown inner encodings, so during a rolling upgrade or after rollback it cannot read/clone rowsets written by an upgraded BE with `PLAIN_ENCODING_V3` (also applies to the dict word/fallback path set in `ScalarColumnWriter`). Please keep the default on the old encoding until the cluster minimum BE version supports V3, or add an explicit compatibility mechanism and tests for mixed-version/rollback behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531020456
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feat](be) add PLAIN_ENCODING_V3 binary plain page with contiguous data + lengths trailer [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63570:
URL: https://github.com/apache/doris/pull/63570#issuecomment-4531406612
<details>
<summary>TPC-H: <b>Total hot run time: 31523 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7f21734b8dcbbbf0b7d1992bda9c91c04ec17641, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17659 4105 4078 4078
q2 q3 10796 1457 806 806
q4 4770 483 354 354
q5 9322 2296 2117 2117
q6 364 173 137 137
q7 976 781 647 647
q8 9578 1754 1611 1611
q9 7034 5039 4963 4963
q10 6484 2255 1902 1902
q11 443 279 256 256
q12 689 422 297 297
q13 18290 3370 2746 2746
q14 269 258 235 235
q15 q16 831 803 705 705
q17 925 900 927 900
q18 6848 5900 5643 5643
q19 1215 1242 1062 1062
q20 519 403 270 270
q21 5633 2521 2485 2485
q22 442 353 309 309
Total cold run time: 103087 ms
Total hot run time: 31523 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4385 4295 4284 4284
q2 q3 4528 4984 4395 4395
q4 2118 2234 1428 1428
q5 4472 4353 5541 4353
q6 258 203 149 149
q7 2113 1859 1656 1656
q8 2611 2241 2417 2241
q9 8056 8110 8044 8044
q10 4868 4873 4343 4343
q11 598 413 379 379
q12 776 777 583 583
q13 3206 3634 2890 2890
q14 327 429 297 297
q15 q16 734 728 646 646
q17 1387 1354 1345 1345
q18 8107 7480 7116 7116
q19 1094 1103 1159 1103
q20 2240 2238 1947 1947
q21 5389 4705 4525 4525
q22 517 456 403 403
Total cold run time: 57784 ms
Total hot run time: 52127 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org