HappenLee commented on PR #66477: URL: https://github.com/apache/doris/pull/66477#issuecomment-5695534122
**[P1] Preserve the probe-side IDENTITY layout through Nested Loop Join** The broadcast HashJoin fix in `6d9bcebcb903e98315fd82a4dd93bf8046489cdf` addresses the earlier comment, but the same layout propagation issue remains for Nested Loop Join at commit `8774b5d426d4477a5f2d822201ec9509218af858`. [ChildOutputPropertyDeriver.visitPhysicalNestedLoopJoin()](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java#L325-L331) preserves the left child's distribution. However, `NestedLoopJoinNode` inherits [PlanNode.getStorageDistributionHashType()](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java#L1171-L1183), which returns `null` if its children have different hash types. A reduced plan illustrating the inconsistency is: ```text Bucket-shuffle HashJoin on a.id = c.id LocalExchange BUCKET_HASH_SHUFFLE NestedLoopJoin CROSS_JOIN Scan a: IDENTITY(a.id) Broadcast Exchange for b: default CRC32 label Bucket-shuffle Exchange for c: IDENTITY(c.id) ``` The optimizer retains the left-side IDENTITY bucket layout, but the translated NLJ reports `null` for its IDENTITY/CRC32 children. [LocalExchangeNode](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/fe/fe-core/src/main/java/org/apache/doris/planner/LocalExchangeNode.java#L54-L68) then keeps its default CRC32 algorithm. The plan above is a reduced tree inferred from the code path, not an EXPLAIN captured from a running query. I reproduced the metadata loss with a focused FE unit test: construct a real `NestedLoopJoinNode(CROSS_JOIN)` with mocked leaf nodes returning IDENTITY and CRC32, wrap it in a real `LocalExchangeNode(BUCKET_HASH_SHUFFLE)`, and assert IDENTITY. The result is: ```text expected: <IDENTITY> but was: <CRC32> ``` When a BE owns only some buckets, CRC32 recomputation can select a bucket absent from that BE's local bucket map even though the input was correctly placed using IDENTITY. The [local exchanger's row-count check](https://github.com/apache/doris/blob/8774b5d426d4477a5f2d822201ec9509218af858/be/src/exec/exchange/local_exchanger.cpp#L209-L235) then reports `Rows mismatched! Data may be lost...`. This runtime consequence is based on the code path; I have not reproduced it with a multi-BE SQL test. Please propagate the probe hash type for NLJ variants that preserve the probe distribution and add an NLJ-to-bucket-join regression. Passing the already-derived distribution metadata through translation would also reduce the risk of the Nereids property derivation and PlanNode subtree inference disagreeing about an operator's output layout. -- 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]
