dongjoon-hyun commented on PR #58681: URL: https://github.com/apache/spark/pull/58681#issuecomment-5615352605
Thank you for the PR, @ulysses-you. The direction makes sense to me. A few comments: 1. The PR description says "Yes, within unreleased branches", but `partition.filter.enabled` has defaulted to `false` since 4.0.0 and the two ordering configs since 4.2.0 (released). This is a user-facing behavior change relative to released versions. Could you update the description? (The migration guide entries are correct.) 2. Several existing tests are now pinned to `partition.filter.enabled=false` (SPARK-42038 DPP, SPARK-57881 union, SPARK-55535, SPARK-59050, ...). Since they used to cover the default path, could we parameterize them over `Seq(true, false)` with the expected partition counts for `true`, at least for the DPP and union cases? That would also verify the claim that the new default never adds shuffles. 3. nit: In the migration guide, "a join that preserves the left side's rows" is not precise for LeftAnti/ExistenceJoin. Maybe "a join that keeps or tests every left row", and mention that an equi CROSS join is treated like INNER. cc @peter-toth @szehon-ho @pan3793 since the filtering for more join types (SPARK-59199) and the ordering derivation (SPARK-56241) are recent. -- 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]
