You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "tianhanhu-db (via GitHub)" <gi...@apache.org> on 2023/10/09 18:50:51 UTC

Re: [PR] [SPARK-44919][AVRO] Avro connector: convert a union of a single primitive type to a StructType [spark]

tianhanhu-db commented on code in PR #42618:
URL: https://github.com/apache/spark/pull/42618#discussion_r1350692984


##########
connector/avro/src/main/scala/org/apache/spark/sql/avro/SchemaConverters.scala:
##########
@@ -142,18 +143,30 @@ object SchemaConverters {
         if (avroSchema.getTypes.asScala.exists(_.getType == NULL)) {
           // In case of a union with null, eliminate it and make a recursive call
           val remainingUnionTypes = AvroUtils.nonNullUnionBranches(avroSchema)
-          if (remainingUnionTypes.size == 1) {
-            toSqlTypeHelper(remainingUnionTypes.head, existingRecordNames, avroOptions)
-              .copy(nullable = true)
-          } else {
-            toSqlTypeHelper(
-              Schema.createUnion(remainingUnionTypes.asJava),
-              existingRecordNames,
-              avroOptions).copy(nullable = true)
-          }
+          toSqlTypeHelper(
+            Schema.createUnion(remainingUnionTypes.asJava),
+            existingRecordNames,
+            avroOptions).copy(nullable = true)
         } else avroSchema.getTypes.asScala.map(_.getType).toSeq match {
           case Seq(t1) =>
-            toSqlTypeHelper(avroSchema.getTypes.get(0), existingRecordNames, avroOptions)
+            // If spark.sql.avro.alwaysConvertUnionToStructType is set to false (default),
+            // we convert Avro union with a single primitive type into a primitive Spark type
+            // instead of a StructType.
+            if (!SQLConf.get.avroAlwaysConvertUnionToStruct) {
+              toSqlTypeHelper(avroSchema.getTypes.get(0), existingRecordNames, avroOptions)

Review Comment:
   The major problem is that the check is meant to safeguard that the top level Avro schema is either an Avro
   record or a complex union. 
   If we add `case t: AtomicType => StructType(StructField("value", t, nullable = true))`, it loosen the restriction and the top level schema could just be an integer or something.



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