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]

Reply via email to