You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "morningman (via GitHub)" <gi...@apache.org> on 2026/10/15 15:01:45 UTC
[PR] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
morningman opened a new pull request, #69142:
URL: https://github.com/apache/doris/pull/69142
### What problem does this PR solve?
Issue Number: None
Related PR: #68921
Problem Summary:
**In short.** `ColumnVarbinary` has no CRC32 / CRC32C hash, so every exchange that partitions rows by a VARBINARY value fails. Since #68786 reads Iceberg BINARY and FIXED as VARBINARY, MERGE INTO an Iceberg table with an `identity` or `truncate` partition on a binary column fails, and #68921 has to leave INSERT into such a table on random writes. This PR hashes VARBINARY values by their bytes, exactly as `ColumnString` does, and lets the Iceberg connector route binary partition values on backends that can hash them.
**Background.** An exchange that partitions rows hashes the partition key of each row with CRC32 (`update_crcs_with_value`, `update_crc_with_value`) or CRC32C (`update_crc32c_batch`, `update_crc32c_single`): a shuffle join on its keys, an Iceberg INSERT routed by the transformed partition value (#68921), and MERGE INTO an Iceberg table, which routes the rows it writes through the same Iceberg partition function. Array, map and struct keys hash their elements through the same methods. `ColumnVarbinary` inherits these methods from `IColumn`, where they throw `NOT_IMPLEMENTED_ERROR`.
**The problem, and what it cost.** On a local single-BE cluster with an Iceberg REST catalog (tables created by Spark, `parallel_pipeline_task_num = 8`):
| Statement | master | with this PR |
|---|---|---|
| MERGE INTO a table partitioned by `identity(binary_key)` or `truncate(1, binary_key)`, with a `numbers()` source | fails: `update_crc32c_batch is not supported for ColumnVarbinary` | succeeds; the rows match what Spark reads back |
| INSERT OVERWRITE of 10000 rows into four `identity(binary_key)` partitions from a `numbers()` source | random writes: 8 data files (measured with this PR at `be_exec_version` 18, the plan master makes) | routed: 4 data files, one per partition |
| `SELECT ... JOIN [shuffle] ... ON l.a = r.a` on `ARRAY<VARBINARY>` keys | fails: `update_crc32c_single is not supported for ColumnVarbinary` | same rows as `JOIN [broadcast]` |
A scalar VARBINARY comparison is rejected during analysis, so a scalar VARBINARY never becomes a join key; the shuffle join only reaches the exchange through a nested type.
**How this PR fixes it.**
- **BE.** `ColumnVarbinary` implements the four CRC methods by hashing each value's bytes, with and without a null map, as `ColumnString` does, so the same bytes get the same hash in both columns. SipHash and xxHash (`update_hash_with_value`, `update_hashes_with_value`, `update_xxHash_with_value`) stay unsupported, and so does tablet routing of VARBINARY.
- **Mixed versions.** BE exec version 19 marks the capability and FE `max_be_exec_version` moves to 19. `IcebergWritePlanProvider.getWriteDistribution` routes a BINARY or FIXED transformed partition value only when the statement's exec version is at least 19; below it the INSERT keeps random writes, as on master today. `bucket(n, binary_key)` yields an INT and is routed from version 18 as before. MERGE needs no FE change.
**Results.**
- **BE UT.** `ColumnVarbinaryTest.ShuffleHashesMatchStringOfTheSameBytes` checks the four methods against `ColumnString` on empty, NUL, invalid UTF-8 and inline / out-of-line values, with and without a null map and on a sub-range; `ArrayOfVarbinaryHashesLikeArrayOfString` does the same through `ColumnArray` with a NULL element; `ValueHashesAreNotSupported` keeps SipHash and xxHash unsupported. `ExternalTableSinkBinaryHashPartitionerTest.IcebergTransformHashesBinaryPartitionValues` routes `identity` and `truncate[1]` binary values with CRC32 and CRC32C, so equal partition values share a channel. On master the first two and the partitioner case fail with `NOT_IMPLEMENTED_ERROR`, since the methods are inherited from `IColumn`. The three suites (36 cases) pass under ASAN; clang-tidy reports nothing on the changed files.
- **FE UT.** `IcebergWritePlanProviderTest#getWriteDistributionRoutesBinaryPartitionValuesOnlyOnBackendsThatHashThem`: identity / truncate on BINARY, identity on FIXED and a mixed INT + BINARY spec keep random writes at exec version 18 and are routed at 19; `bucket(16, binary_key)` is routed at 18. All 1568 tests of fe-connector-iceberg pass.
- **Regression.** `test_iceberg_write_binary_partitions` now checks the data files of the multi-writer INSERT and INSERT OVERWRITE (4 for identity, 1 for truncate(1), which folds the keys into one partition) and adds MERGE INTO identity / truncate binary partitions, compared with Spark. `test_varbinary_sql_support` adds a broadcast and a shuffle join on `ARRAY<VARBINARY>` keys with `parallel_pipeline_task_num = 4`; both return the same 12 rows, and the shuffle plan hash-partitions both sides by the array. Both suites pass on a local single-BE cluster against the Iceberg REST docker environment, where their `.out` files were generated; only the new blocks were added.
- **Mixed versions.** On the same cluster, with `be_exec_version` set to 18 the INSERT OVERWRITE above writes 8 data files (random writes), and with 19 it writes 4.
### Release note
MERGE INTO an Iceberg table with an `identity` or `truncate` partition on a binary column no longer fails, and an INSERT into such a table writes each partition from one writer. A shuffle join on `ARRAY<VARBINARY>` keys no longer fails.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_iceberg_write_binary_partitions` and `test_varbinary_sql_support`, run on a local single-BE cluster against the Iceberg REST docker environment; the `.out` files were generated there.
- [x] Unit Test
- BE: `ColumnVarbinaryTest`, `ExternalTableSinkHashPartitionerTest`, `ExternalTableSinkBinaryHashPartitionerTest` (36 cases, ASAN).
- FE: all 1568 tests of fe-connector-iceberg, including `IcebergWritePlanProviderTest`.
- [ ] 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.
- [x] Yes. <!-- Explain the behavior change -->
- Exchanges can partition rows by VARBINARY values; Iceberg INSERT routes binary partition values on backends at exec version 19.
- Does this need documentation?
- [x] 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
--
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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6195206080
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6197575810
<details>
<summary>TPC-H: <b>Total hot run time: 29526 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 d62762ecee782903eae16050f1d2989afe81a1c9, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17577 5681 5515 5515
q2 2132 315 272 272
q3 10240 1434 843 843
q4 4674 470 341 341
q5 7434 802 531 531
q6 182 183 156 156
q7 773 797 627 627
q8 9255 1329 1107 1107
q9 5530 4224 4250 4224
q10 6814 1282 1047 1047
q11 454 249 233 233
q12 636 398 289 289
q13 18096 3593 2989 2989
q14 275 280 253 253
q15 q16 762 735 689 689
q17 1226 757 571 571
q18 7243 6250 6185 6185
q19 1115 981 605 605
q20 396 341 229 229
q21 4986 2519 2666 2519
q22 406 349 301 301
Total cold run time: 100206 ms
Total hot run time: 29526 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 6666 6636 6702 6636
q2 726 641 548 548
q3 5489 5791 5306 5306
q4 2014 2158 1552 1552
q5 5729 5690 5852 5690
q6 270 191 150 150
q7 2271 2018 1735 1735
q8 3208 2760 2755 2755
q9 7985 7991 7874 7874
q10 3650 3598 3244 3244
q11 617 412 363 363
q12 662 725 504 504
q13 3291 3598 2999 2999
q14 283 289 323 289
q15 q16 671 670 630 630
q17 7743 7014 6903 6903
q18 13146 12280 13102 12280
q19 914 787 824 787
q20 2179 2158 1925 1925
q21 5831 4805 4926 4805
q22 506 442 413 413
Total cold run time: 73851 ms
Total hot run time: 67388 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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6197864724
<details>
<summary>ClickBench: <b>Total hot run time: 24.25 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit d62762ecee782903eae16050f1d2989afe81a1c9, data reload: false
query1 0.00 0.00 0.00
query2 0.09 0.05 0.05
query3 0.27 0.12 0.12
query4 1.61 0.12 0.13
query5 0.24 0.22 0.25
query6 1.17 0.68 0.68
query7 0.04 0.01 0.01
query8 0.05 0.04 0.04
query9 0.39 0.36 0.36
query10 0.56 0.56 0.59
query11 0.20 0.15 0.14
query12 0.19 0.14 0.14
query13 0.48 0.53 0.49
query14 1.03 1.02 1.01
query15 0.63 0.61 0.59
query16 0.35 0.35 0.31
query17 1.11 1.14 1.16
query18 0.22 0.20 0.20
query19 2.08 1.98 1.93
query20 0.01 0.01 0.01
query21 15.49 0.25 0.15
query22 4.41 0.06 0.05
query23 16.23 0.32 0.13
query24 3.19 0.62 0.35
query25 0.13 0.08 0.04
query26 1.28 0.25 0.15
query27 0.04 0.04 0.03
query28 2.34 0.61 0.36
query29 12.51 4.18 3.33
query30 0.28 0.13 0.14
query31 2.77 0.56 0.33
query32 3.22 0.60 0.52
query33 3.26 3.08 3.23
query34 15.53 4.04 3.41
query35 3.37 3.32 3.35
query36 0.55 0.42 0.45
query37 0.10 0.06 0.06
query38 0.05 0.04 0.03
query39 0.04 0.03 0.03
query40 0.17 0.14 0.14
query41 0.08 0.02 0.02
query42 0.04 0.02 0.02
query43 0.04 0.03 0.04
Total cold run time: 95.84 s
Total hot run time: 24.25 s
```
</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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6200709518
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69142_d62762ecee782903eae16050f1d2989afe81a1c9_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69142_d62762ecee782903eae16050f1d2989afe81a1c9_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 77.09% (35721/46337) |
| Line Coverage | 62.39% (405060/649263) |
| Region Coverage | 58.77% (341965/581889) |
| Branch Coverage | 59.70% (157840/264399) |
--
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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman merged PR #69142:
URL: https://github.com/apache/doris/pull/69142
--
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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6200926240
# FE Regression Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69142_d62762ecee782903eae16050f1d2989afe81a1c9_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69142_d62762ecee782903eae16050f1d2989afe81a1c9_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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6197771397
<details>
<summary>TPC-DS: <b>Total hot run time: 152111 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 d62762ecee782903eae16050f1d2989afe81a1c9, data reload: false
query5 4301 587 448 448
query6 422 212 200 200
query7 4818 444 229 229
query8 345 164 155 155
query9 8731 3928 3966 3928
query10 485 310 271 271
query11 6095 3822 3550 3550
query12 150 92 86 86
query13 1282 432 325 325
query14 6149 4036 3726 3726
query14_1 3521 3560 3511 3511
query15 207 204 181 181
query16 971 434 351 351
query17 855 655 545 545
query18 2429 419 321 321
query19 202 167 145 145
query20 83 83 82 82
query21 209 144 112 112
query22 13058 13120 12817 12817
query23 13588 12827 12463 12463
query23_1 12571 12555 12471 12471
query24 7113 836 463 463
query24_1 475 475 480 475
query25 538 419 369 369
query26 1285 261 139 139
query27 2756 438 267 267
query28 4645 1959 1971 1959
query29 1567 623 453 453
query30 304 221 190 190
query31 1023 936 816 816
query32 159 102 86 86
query33 517 308 252 252
query34 918 856 491 491
query35 751 763 648 648
query36 799 808 719 719
query37 136 102 92 92
query38 1816 1741 1710 1710
query39 796 796 752 752
query39_1 751 731 755 731
query40 221 116 96 96
query41 72 70 70 70
query42 92 85 85 85
query43 362 385 331 331
query44 1310 698 710 698
query45 183 177 208 177
query46 840 968 556 556
query47 3031 2949 2944 2944
query48 304 318 221 221
query49 591 404 306 306
query50 666 275 209 209
query51 10450 10387 10581 10387
query52 87 83 69 69
query53 185 205 148 148
query54 244 197 185 185
query55 81 68 65 65
query56 232 212 209 209
query57 1631 1591 1558 1558
query58 289 275 256 256
query59 2146 2133 1894 1894
query60 279 247 245 245
query61 154 152 147 147
query62 390 341 287 287
query63 204 158 159 158
query64 2729 945 816 816
query65 3327 3277 3242 3242
query66 1787 421 313 313
query67 20533 20269 20018 20018
query68 3045 1022 613 613
query69 388 298 272 272
query70 895 774 760 760
query71 293 232 209 209
query72 2876 2540 2338 2338
query73 521 530 289 289
query74 4645 4475 4327 4327
query75 2333 2306 1919 1919
query76 2307 1026 608 608
query77 344 391 277 277
query78 9734 9777 9143 9143
query79 957 923 530 530
query80 872 419 344 344
query81 570 323 275 275
query82 756 142 116 116
query83 305 205 181 181
query84 387 120 106 106
query85 1000 415 365 365
query86 356 242 225 225
query87 1986 1953 1822 1822
query88 3613 2700 2685 2685
query89 337 297 261 261
query90 1739 183 183 183
query91 166 152 126 126
query92 103 80 86 80
query93 1016 1002 584 584
query94 560 306 295 295
query95 575 412 313 313
query96 656 524 236 236
query97 2456 2420 2298 2298
query98 163 154 147 147
query99 782 755 653 653
Total cold run time: 235721 ms
Total hot run time: 152111 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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6195208050
/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] [fix](be) Hash VARBINARY values in shuffle and partition exchanges [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on PR #69142:
URL: https://github.com/apache/doris/pull/69142#issuecomment-6195207438
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