HappenLee commented on PR #66477: URL: https://github.com/apache/doris/pull/66477#issuecomment-5617546808
**[P1] Normalize storage hash metadata when converting to EXECUTION_BUCKETED** [ChildrenPropertiesRegulator.calAnotherSideRequired()](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java#L956-L963) unconditionally copies `notNeedShuffleSideOutput.getHashType()`, including when the requested shuffle type is `EXECUTION_BUCKETED`. This can produce an `EXECUTION_BUCKETED` property labeled IDENTITY when an identity bucket join is downgraded to an ordinary shuffle, for example with `enable_bucket_shuffle_join=false`: the [downgrade branches](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java#L616-L634) pass the original storage spec into this helper. However, `PhysicalPlanTranslator` translates EXECUTION_BUCKETED into ordinary `HASH_PARTITIONED`, and the [BE exchange sink](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/be/src/exec/operator/exchange_sink_operator.cpp#L126-L134) uses CRC32C or CRC32 according to the query option. It does not use the table's IDENTITY algorithm for this exchange. A reduced problematic property combination is: ```text Inner hash join on x.id = y.id Subplan X: EXECUTION_BUCKETED, retained IDENTITY label Subplan Y: EXECUTION_BUCKETED, CRC32 label ``` Both sides physically use the same execution shuffle algorithm. The regulator's both-EXECUTION_BUCKETED branch checks key order, but the new [DistributionSpecHash.merge() assertion](https://github.com/apache/doris/blob/e920dbd3f53c39152b0a9434a1ced47538db38a4/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecHash.java#L147-L150) rejects the different retained storage labels. I reproduced the failure in a focused FE unit test by constructing these two distribution specs, passing them to the real `ChildOutputPropertyDeriver`, and deriving the output of an inner hash join. It throws: ```text can not merge distribution specs with different hash types: IDENTITY vs CRC32 ``` This is a property-derivation unit reproduction with directly constructed child properties and a mocked join type, not an end-to-end SQL reproduction. The producer of the inconsistent properties is the downgrade/helper path above. Please normalize the storage hash metadata when transitioning to EXECUTION_BUCKETED, and define hash-type equality according to the relevant distribution kind. Merely removing the merge assertion would leave the metadata inconsistent with execution. A regression should combine an identity-origin execution-shuffled subplan with a CRC32-origin execution-shuffled subplan. -- 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]
