andygrove commented on PR #6005: URL: https://github.com/apache/datafusion-comet/pull/6005#issuecomment-5783255228
#5421 is merged now, so the prerequisite here is met. Could you merge main and drop the carried commits? That would leave just `f509e5db8` plus the regression test, which makes the routing change reviewable on its own, and then this can come out of draft. One thing worth adding to the rationale: this fixes more than the aggregate-overflow case. `applyCometShuffle` picks native or columnar per exchange with no harmonization across the plan, so the two sides of a join can end up with different shuffle implementations — native murmur3 on one side, Spark's `partitionIdExpression` on the other. On a `DECIMAL(38,0)` key those disagree and the join silently drops rows; I reproduced zero rows against Spark's four on main yesterday. Your guard covers that too, since once the native path declines the wide decimal both sides use Spark's partitioner and agree again. Details are in #3079. -- 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]
