HappenLee commented on PR #66477: URL: https://github.com/apache/doris/pull/66477#issuecomment-5617546404
**[P1] Preserve the probe-side IDENTITY layout through broadcast joins** At commit `e920dbd3f53c39152b0a9434a1ced47538db38a4`, the new [PlanNode.getStorageDistributionHashType()](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java#L1161-L1173) returns `null` whenever its children report different hash types. `HashJoinNode` inherits this implementation, but a broadcast join preserves the probe side's storage layout; the build side's hash label should not determine that layout. A reduced plan illustrating the issue is: ```text Bucket-shuffle join on a.id = c.id LocalExchange BUCKET_HASH_SHUFFLE Broadcast hash join on a.id = b.id Scan a: IDENTITY Broadcast Exchange for b: default CRC32 label Bucket-shuffle Exchange for c: IDENTITY ``` The broadcast join reports `null` for the `IDENTITY`/`CRC32` children. [LocalExchangeNode](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/fe/fe-core/src/main/java/org/apache/doris/planner/LocalExchangeNode.java#L63-L68) then leaves its hash type at the default `CRC32`, although the probe data still follows the IDENTITY bucket layout. I reproduced this with a focused FE unit test: construct a real broadcast `HashJoinNode` with mocked leaf nodes returning IDENTITY and CRC32, wrap it in a real `LocalExchangeNode(BUCKET_HASH_SHUFFLE)`, and assert that the resulting hash type is IDENTITY. The test fails with: ```text expected: <IDENTITY> but was: <CRC32> ``` On a multi-BE layout, CRC32 recomputation can select buckets absent from the current BE's local bucket map. The [local exchanger's row-count check](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/be/src/exec/exchange/local_exchanger.cpp#L209-L235) then returns `Rows mismatched! Data may be lost...`. This runtime consequence follows from the code path; I have not reproduced it with an end-to-end multi-BE SQL test. The BE-native local-exchange path also derives the fragment hash type from the plan root, so the same loss can leave its bucket partitioner at CRC32. Please derive the hash type from the distribution actually preserved by each operator, with broadcast joins inheriting the probe side, and add a broadcast-join → bucket-join regression covering both FE-planned and BE-native local exchange. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
