You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "jacktengg (via GitHub)" <gi...@apache.org> on 2026/03/23 10:29:19 UTC
[PR] [chore](topn) check column type before merging lazy materialization columns [doris]
jacktengg opened a new pull request, #61633:
URL: https://github.com/apache/doris/pull/61633
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Check column type before merging lazy materialization columns, to avoid coredump in case of any column type mismatch.
### 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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #61633:
URL: https://github.com/apache/doris/pull/61633
--
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4109778058
<details>
<summary>TPC-DS: <b>Total hot run time: 168366 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 67fdbb5e221e07555deff4d44d4d2482a945c224, data reload: false
query5 4340 661 511 511
query6 339 236 219 219
query7 4218 466 264 264
query8 346 241 225 225
query9 8733 2694 2678 2678
query10 505 375 335 335
query11 6978 5071 4870 4870
query12 189 134 141 134
query13 1275 475 357 357
query14 5809 3706 3500 3500
query14_1 2854 2794 2811 2794
query15 199 191 183 183
query16 981 458 453 453
query17 1125 735 638 638
query18 2461 452 360 360
query19 225 217 185 185
query20 140 129 130 129
query21 218 142 113 113
query22 13301 14034 14715 14034
query23 16260 15789 15589 15589
query23_1 15913 15463 15416 15416
query24 7380 1612 1240 1240
query24_1 1234 1231 1249 1231
query25 600 464 407 407
query26 1245 264 145 145
query27 2773 478 293 293
query28 4493 1821 1836 1821
query29 864 571 481 481
query30 296 231 194 194
query31 1014 952 856 856
query32 89 74 76 74
query33 509 331 294 294
query34 899 876 525 525
query35 653 666 590 590
query36 1067 1144 993 993
query37 141 97 80 80
query38 2929 2928 2868 2868
query39 840 846 811 811
query39_1 806 792 794 792
query40 237 151 134 134
query41 65 59 61 59
query42 257 259 296 259
query43 235 250 224 224
query44
query45 204 191 184 184
query46 868 986 607 607
query47 2087 2119 2045 2045
query48 322 315 228 228
query49 642 484 406 406
query50 716 305 226 226
query51 4063 4067 4018 4018
query52 263 265 256 256
query53 290 336 281 281
query54 324 278 277 277
query55 92 90 79 79
query56 326 331 310 310
query57 1928 1798 1708 1708
query58 288 278 292 278
query59 2805 2929 2762 2762
query60 348 332 328 328
query61 167 159 155 155
query62 625 605 538 538
query63 313 281 281 281
query64 5148 1293 1022 1022
query65
query66 1459 464 363 363
query67 24247 24321 24262 24262
query68
query69 406 319 290 290
query70 993 937 940 937
query71 335 307 297 297
query72 2920 2731 2433 2433
query73 533 541 324 324
query74 9605 9610 9453 9453
query75 2870 2791 2461 2461
query76 2282 1026 682 682
query77 360 382 315 315
query78 10873 11078 10471 10471
query79 1085 801 571 571
query80 800 629 534 534
query81 509 261 226 226
query82 1358 156 118 118
query83 343 258 248 248
query84 257 124 97 97
query85 958 505 474 474
query86 411 315 310 310
query87 3120 3099 3011 3011
query88 3574 2675 2660 2660
query89 439 377 350 350
query90 1883 190 181 181
query91 176 165 142 142
query92 77 77 73 73
query93 919 824 487 487
query94 479 325 291 291
query95 601 400 319 319
query96 656 522 226 226
query97 2470 2502 2417 2417
query98 245 221 222 221
query99 1002 992 915 915
Total cold run time: 249473 ms
Total hot run time: 168366 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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4109569674
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4109570555
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4109720444
<details>
<summary>TPC-H: <b>Total hot run time: 26742 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 67fdbb5e221e07555deff4d44d4d2482a945c224, 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 17598 4660 4325 4325
q2 q3 10646 767 509 509
q4 4680 357 256 256
q5 7559 1211 1017 1017
q6 172 174 149 149
q7 766 851 677 677
q8 9299 1454 1297 1297
q9 4849 4720 4731 4720
q10 6249 1925 1647 1647
q11 455 264 263 263
q12 695 593 465 465
q13 18017 2894 2159 2159
q14 233 236 224 224
q15 q16 748 730 664 664
q17 729 869 398 398
q18 6031 5382 5245 5245
q19 1249 969 610 610
q20 525 495 377 377
q21 4656 1833 1411 1411
q22 548 385 329 329
Total cold run time: 95704 ms
Total hot run time: 26742 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 4785 4617 4551 4551
q2 q3 3858 4317 3872 3872
q4 869 1212 815 815
q5 4083 4388 4378 4378
q6 191 188 147 147
q7 1785 1679 1561 1561
q8 2477 2762 2581 2581
q9 7625 7592 7459 7459
q10 3753 3979 3582 3582
q11 540 441 421 421
q12 493 605 455 455
q13 2721 3197 2333 2333
q14 282 300 280 280
q15 q16 728 772 722 722
q17 1166 1373 1358 1358
q18 7148 6797 6532 6532
q19 875 896 900 896
q20 2113 2143 2007 2007
q21 3967 3494 3558 3494
q22 463 428 374 374
Total cold run time: 49922 ms
Total hot run time: 47818 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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4114802714
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #61633:
URL: https://github.com/apache/doris/pull/61633#discussion_r2975906599
##########
be/src/exec/operator/materialization_opertor.cpp:
##########
@@ -80,6 +80,14 @@ Status MaterializationSharedState::merge_multi_response() {
}
}
+ // return error if any column in response block is not compatible with source block column
+ for (int k = 0; k < response_blocks[i].columns(); ++k) {
+ const auto& resp_col_type = response_blocks[i].get_datatype_by_position(k);
+ for (const auto& [_, source_block_rows] : block_maps) {
+ RETURN_IF_ERROR(resp_col_type->check_column(
Review Comment:
**[AGENTS.md Violation] Defensive programming: should use `DORIS_CHECK` instead of `RETURN_IF_ERROR`**
Per AGENTS.md:
> *Assert correctness only—never use defensive programming with `if` or similar constructs. Any `if` check for errors must have a clearly known inevitable failure path (not speculation).*
Both `response_blocks[i]` and the deserialized `partial_block` in `block_maps` are derived from the same `SlotDescriptor` set — `response_blocks` is built in `init_multi_requests()` (line 258) from `slots_res`, and the remote BE constructs its response block from the same slots serialized into the protobuf request (line 255). Under normal operation, their column types are guaranteed to match.
The PR description uses speculative language ("in case of any column type mismatch") and no issue is linked demonstrating a real failure path. A column type mismatch here would indicate a fundamental invariant violation (corrupted schema propagation), and per AGENTS.md the correct response is to crash, not silently return an error.
Suggested change:
```cpp
for (int k = 0; k < response_blocks[i].columns(); ++k) {
const auto& resp_col_type = response_blocks[i].get_datatype_by_position(k);
for (const auto& [_, source_block_rows] : block_maps) {
DORIS_CHECK(resp_col_type->check_column(
*source_block_rows.first.get_by_position(k).column).ok());
}
}
```
This is not inside a performance-sensitive inner loop (it iterates over columns × backends, both small), so `DORIS_CHECK` is appropriate over `DCHECK`.
--
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4114802616
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "zclllyybb (via GitHub)" <gi...@apache.org>.
zclllyybb commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4111588616
/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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4110090570
# BE UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/67fdbb5e221e07555deff4d44d4d2482a945c224_67fdbb5e221e07555deff4d44d4d2482a945c224/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/67fdbb5e221e07555deff4d44d4d2482a945c224_67fdbb5e221e07555deff4d44d4d2482a945c224/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.80% (19851/37597) |
| Line Coverage | 36.28% (185299/510735) |
| Region Coverage | 32.54% (143546/441148) |
| Branch Coverage | 33.74% (62854/186285) |
--
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] [chore](topn) check column type before merging lazy materialization columns [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61633:
URL: https://github.com/apache/doris/pull/61633#issuecomment-4112065003
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61633_67fdbb5e221e07555deff4d44d4d2482a945c224_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61633_67fdbb5e221e07555deff4d44d4d2482a945c224_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.66% (26372/36801) |
| Line Coverage | 54.43% (277075/509027) |
| Region Coverage | 51.72% (230247/445200) |
| Branch Coverage | 53.14% (99253/186773) |
--
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