ulysses-you commented on PR #58044: URL: https://github.com/apache/spark/pull/58044#issuecomment-5350578776
Thanks @peter-toth for the careful re-check and for pinning down the qualifier discriminator! **Root cause summary: nullability and qualifier are two real manifestations of the same issue** `UnionExec.outputPartitioning` compares the children's partitionings to decide whether they can be passed through. That comparison is structural — `AttributeReference.equals` compares `name`, `dataType`, `nullable`, `metadata`, and `qualifier` — but a child's `outputPartitioning` is not guaranteed to be expressed in the same attributes as its own `output`. It can differ in any of those fields: - **Nullability** (a real cause, not just a hypothesis): `FilterExec` passes its child's partitioning through verbatim while narrowing its own `output` via `IsNotNull`, so the partition key stays nullable in the partitioning but becomes non-nullable in the output. The new DataFrame test isolates exactly this and fails on the pre-fix base — the discriminator assertions confirm the two attributes differ *only* in nullability, and the union falls back to `UnknownPartitioning` (3 shuffles) — then passes after the fix. It simply isn't reachable through the normal SQL path because `PushPredicateThroughNonJoin` relocates `IsNotNull` below the shuffle; the test disables `PushDownPredicates` to keep `FilterExec` directly above the shuffle. - **Qualifier**: a partitioning built inside a view/subquery carries that relation's qualifier (e.g. `[t1]`), while the outer `Project` re-exposes the column with the subquery qualifier (e.g. `[__auto_generated_subquery_name]`). This is what the SQL test discriminates on. Before this PR, `prepareOutputPartitioning` anchored on the first child and left its partitioning un-remapped, so *either* difference made the first child compare unequal to the remapped siblings and the union fell back to `UnknownPartitioning`. **What changed (this commit)** - **Added a dedicated nullability test** (DataFrame). It isolates the nullability cause by disabling `PushDownPredicates` so `FilterExec` stays directly above the shuffle, and pins the discriminator: the output and partitioning attributes must differ *only* in nullability (same `exprId` and `qualifier`), not in qualifier. - Broadened the `prepareOutputPartitioning` comment to name the full set of fields `AttributeReference.equals` compares (name, nullability, metadata, qualifier) instead of singling out nullability. - Renamed the existing SQL test to "compares children in the union's attribute space" to reflect that its discriminator is the qualifier. - Added `assert(hashPartitioning.expressions == Seq(unionExec.head.output.head))` to both tests, pinning that the propagated partitioning is expressed in the union's own output attributes. -- 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]
