viirya commented on a change in pull request #29587:
URL: https://github.com/apache/spark/pull/29587#discussion_r480239099



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveUnion.scala
##########
@@ -17,29 +17,97 @@
 
 package org.apache.spark.sql.catalyst.analysis
 
+import scala.collection.mutable
+
 import org.apache.spark.sql.AnalysisException
-import org.apache.spark.sql.catalyst.expressions.{Alias, Literal}
+import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute, 
Expression, Literal, NamedExpression, WithFields}
 import org.apache.spark.sql.catalyst.optimizer.CombineUnions
 import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project, 
Union}
 import org.apache.spark.sql.catalyst.rules.Rule
 import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
 import org.apache.spark.sql.util.SchemaUtils
 
 /**
  * Resolves different children of Union to a common set of columns.
  */
 object ResolveUnion extends Rule[LogicalPlan] {
-  private def unionTwoSides(
+  /**
+   * Adds missing fields recursively into given `col` expression, based on the 
target `StructType`.
+   * For example, given `col` as "a struct<a:int, b:int>, b int" and `target` 
as
+   * "a struct<a:int, b:int, c: long>, b int, c string", this method should 
add `a.c` and `c` to
+   * `col` expression.
+   */
+  private def addFields(col: NamedExpression, target: StructType): 
Option[Expression] = {
+    require(col.dataType.isInstanceOf[StructType], "Only support StructType.")
+
+    val resolver = SQLConf.get.resolver
+    val missingFields =
+      StructType.findMissingFields(col.dataType.asInstanceOf[StructType], 
target, resolver)
+    if (missingFields.length == 0) {
+      None
+    } else {
+      Some(addFieldsInto(col, "", missingFields.fields))
+    }
+  }
+
+  private def addFieldsInto(col: Expression, base: String, fields: 
Seq[StructField]): Expression = {
+    var currCol = col
+    fields.foreach { field =>
+      field.dataType match {
+        case dt: AtomicType =>
+          // We need to sort columns in result, because we might add another 
column in other side.
+          // E.g., we want to union two structs "a int, b long" and "a int, c 
string".
+          // If we don't sort, we will have "a int, b long, c string" and "a 
int, c string, b long",
+          // which are not compatible.
+          currCol = WithFields(currCol, s"$base${field.name}", Literal(null, 
dt),
+            sortColumns = true)
+        case st: StructType =>
+          val resolver = SQLConf.get.resolver
+          val colField = currCol.dataType.asInstanceOf[StructType]
+            .find(f => resolver(f.name, field.name))
+          if (colField.isEmpty) {
+            // The whole struct is missing. Add a null.
+            currCol = WithFields(currCol, s"$base${field.name}", Literal(null, 
st),
+              sortColumns = true)
+          } else {
+            currCol = addFieldsInto(currCol, s"$base${field.name}.", st.fields)
+          }
+      }
+    }
+    currCol
+  }
+
+  private def compareAndAddFields(
       left: LogicalPlan,
       right: LogicalPlan,
-      allowMissingCol: Boolean): LogicalPlan = {
+      allowMissingCol: Boolean): (Seq[NamedExpression], Seq[NamedExpression]) 
= {
     val resolver = SQLConf.get.resolver
     val leftOutputAttrs = left.output
     val rightOutputAttrs = right.output
 
-    // Builds a project list for `right` based on `left` output names
+    val aliased = mutable.ArrayBuffer.empty[Attribute]
+
     val rightProjectList = leftOutputAttrs.map { lattr =>
-      rightOutputAttrs.find { rattr => resolver(lattr.name, rattr.name) 
}.getOrElse {
+      val found = rightOutputAttrs.find { rattr => resolver(lattr.name, 
rattr.name) }
+      if (found.isDefined) {
+        val foundDt = found.get.dataType
+        (foundDt, lattr.dataType) match {
+          case (source: StructType, target: StructType)
+              if allowMissingCol && !source.sameType(target) =>
+            // Having an output with same name, but different struct type.
+            // We need to add missing fields.
+            addFields(found.get, target).map { added =>
+              aliased += found.get
+              Alias(added, found.get.name)()
+            }.getOrElse(found.get) // Data type doesn't change. We should add 
fields at other side.
+          case _ =>
+            // Same struct type, or
+            // unsupported: different types, array or map types, or

Review comment:
       Array and map types aren't supported by `WithFields`. I think it is 
still possible to add them to `WithFields`. Once `WithFields` supports these 
types, we can add them here too.




----------------------------------------------------------------
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.

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