[GitHub] [spark] HyukjinKwon commented on a diff in pull request #41554: [SPARK-43781][SQL] Fix IllegalStateException when cogrouping two datasets derived from the same source

["HyukjinKwon (via GitHub)" <gi...@apache.org>]

HyukjinKwon commented on code in PR #41554:
URL: https://github.com/apache/spark/pull/41554#discussion_r1288052991


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/DeduplicateRelations.scala:
##########
@@ -220,7 +220,54 @@ object DeduplicateRelations extends Rule[LogicalPlan] {
           if (attrMap.isEmpty) {
             planWithNewChildren
           } else {
-            planWithNewChildren.rewriteAttrs(attrMap)
+            def rewriteAttrsMatchWithSubPlan(
+                attrs: Seq[Attribute],
+                attrMap: AttributeMap[Attribute],
+                planOutput: Seq[Attribute]): Seq[Attribute] = {
+              val canRewriteAttrs = attrMap.filter(a => planOutput.contains(a._2))
+              attrs.map(attr => canRewriteAttrs.getOrElse(attr, attr))
+            }
+
+            def rewriteOrderMatchWithSubPlan(
+                attrs: Seq[SortOrder],
+                attrMap: AttributeMap[Attribute],
+                planOutput: Seq[Attribute]): Seq[SortOrder] = {
+              val canRewriteAttrs = attrMap.filter(a => planOutput.contains(a._2))
+              attrs.map(attr => {
+                attr.transformWithPruning(_.containsPattern(ATTRIBUTE_REFERENCE)) {
+                  case a: AttributeReference =>
+                    canRewriteAttrs.getOrElse(a, a)
+                }
+                attr
+              })
+            }
+
+            planWithNewChildren match {
+              case c: CoGroup =>
+                // SPARK-43781: CoGroup is a special case, as it has different output attributes

Review Comment:
   What do you mean by "it has different output attributes"?
   
   There are similar cases where it creates new output attributes (not from the child):  `MapPartitionsInR`, `MapPartitionsInRWithArrow`, `MapElements`, `MapGroups`, `FlatMapGroupsWithState`, `FlatMapGroupsInR`, `FlatMapGroupsInR`, `FlatMapGroupsInRWithArrow`, `FlatMapGroupsInPandas`,  `MapInPandas`, `PythonMapInArrow`,  and `FlatMapGroupsInPandasWithState`.
   
   There's another CoGroup operation in Python (`cogroup.apply()` that is `FlatMapCoGroupsInPandas`). Would be great to double check.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org