Kimahriman commented on a change in pull request #32448: URL: https://github.com/apache/spark/pull/32448#discussion_r650522493
########## File path: sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala ########## @@ -150,95 +150,36 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper { assert(fromDDL(interval).toDDL === interval) } - test("find missing (nested) fields") { - val schema = StructType.fromDDL("c1 INT, c2 STRUCT<c3: INT, c4: STRUCT<c5: INT, c6: INT>>") + test("SPARK-35290: Struct merging case insensitive") { + val schema1 = StructType.fromDDL("a1 INT, a2 STRING, nested STRUCT<b1: INT, b2: STRING>") + val schema2 = StructType.fromDDL("A2 STRING, a3 DOUBLE, nested STRUCT<B2: STRING, b3: DOUBLE>") val resolver = SQLConf.get.resolver - val source1 = StructType.fromDDL("c1 INT") - val missing1 = StructType.fromDDL("c2 STRUCT<c3: INT, c4: STRUCT<c5: INT, c6: INT>>") - assert(StructType.findMissingFields(source1, schema, resolver) - .exists(_.sameType(missing1))) + assert(schema1.merge(schema2, resolver).sameType(StructType.fromDDL( + "a1 INT, a2 STRING, nested STRUCT<b1: INT, b2: STRING, b3: DOUBLE>, a3 DOUBLE" + ))) - val source2 = StructType.fromDDL("c1 INT, c3 STRING") - val missing2 = StructType.fromDDL("c2 STRUCT<c3: INT, c4: STRUCT<c5: INT, c6: INT>>") - assert(StructType.findMissingFields(source2, schema, resolver) - .exists(_.sameType(missing2))) - - val source3 = StructType.fromDDL("c1 INT, c2 STRUCT<c3: INT>") - val missing3 = StructType.fromDDL("c2 STRUCT<c4: STRUCT<c5: INT, c6: INT>>") - assert(StructType.findMissingFields(source3, schema, resolver) - .exists(_.sameType(missing3))) - - val source4 = StructType.fromDDL("c1 INT, c2 STRUCT<c3: INT, c4: STRUCT<c6: INT>>") - val missing4 = StructType.fromDDL("c2 STRUCT<c4: STRUCT<c5: INT>>") - assert(StructType.findMissingFields(source4, schema, resolver) - .exists(_.sameType(missing4))) + assert(schema2.merge(schema1, resolver).sameType(StructType.fromDDL( + "a2 STRING, a3 DOUBLE, nested STRUCT<b2: STRING, b3: DOUBLE, b1: INT>, a1 INT" Review comment: Ah yeah it does, `sameType` is just doing a case insensitive comparison so it didn't matter that my manual type was wrong. I'll update to `===` instead and that seems to fix the check ########## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveUnion.scala ########## @@ -181,7 +100,8 @@ object ResolveUnion extends Rule[LogicalPlan] { // like that. We will sort columns in the struct expression to make sure two sides of // union have consistent schema. aliased += foundAttr - Alias(addFields(foundAttr, target), foundAttr.name)() + val targetType = target.merge(source, conf.resolver) Review comment: Hmmm good question, I'm not sure exactly how that would work without adding extra logic to StructType.merge to ignore conflicts. And now that you bring that up I'm starting to think using StructType.merge isn't the best method since it does care about DataType. I just noticed it doesn't handle similar types, so you get errors if you try to merge a float and a double, whereas the normal union just handles that. I might try to rework this again to not use the StructType.merge after all... -- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org