dongjoon-hyun commented on code in PR #57753:
URL: https://github.com/apache/spark/pull/57753#discussion_r3714577409
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/planmerging/PlanMerger.scala:
##########
@@ -700,17 +707,42 @@ class PlanMerger(
// order (npMapping.values would be exprId-hash-ordered).
val unionAttrs = cp.output ++
np.output.map(npMapping).filterNot(cp.outputSet.contains)
+ // The reported key-grouped partitioning / ordering the merged scan must
preserve so BOTH inputs
+ // stay not-worse. Each input reports its own, remapped into cp's relation
space (cp's already
+ // is; np's via npRelationMapping). The two usually agree (same table) but
need not -- differing
+ // best-effort filters can prune different files, and a source may report
per file set. Combine
+ // them into the single report the merge must keep (kGP: they must be
equal; ordering: the
+ // stronger, which satisfies both). None from combine* means the inputs
are INCOMPATIBLE -- no
+ // rebuilt scan could keep both not-worse -- so decline HERE, before
rebuilding, unless the
+ // matching config accepts degrading that dimension.
+ val requiredKeyGroupedPartitioning = combineRequiredKeyGroupedPartitioning(
Review Comment:
`requiredKeyGroupedPartitioning` / `requiredOrdering` seem to carry a double
meaning here. These `Option`s use `.isEmpty` to mean the inputs' reports are
*incompatible*, while the same names are reused for the `Seq` parameters of
`tryBuildMergedDSv2Scan` / `mergeDegradesReporting`, where empty means *no
requirement*. Renaming the `Option`s to `combined*` (and deriving the
`expected*` `Seq`s from them) makes the early-decline condition read correctly
at the use site. Maybe
- `requiredKeyGroupedPartitioning` -> `combinedKeyGroupedPartitioning`
- `requiredOrdering` -> `combinedOrdering`
--
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]