Github user ssimeonov commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21840#discussion_r204243887
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
 ---
    @@ -3858,3 +3858,29 @@ object ArrayUnion {
         new GenericArrayData(arrayBuffer)
       }
     }
    +
    +case class StructCopy(
    +    struct: Expression,
    +    fieldName: String,
    +    fieldValue: Expression) extends Expression with CodegenFallback {
    +
    +  override def children: Seq[Expression] = Seq(struct, fieldValue)
    +  override def nullable: Boolean = struct.nullable
    +
    +  lazy val fieldIndex = 
struct.dataType.asInstanceOf[StructType].fieldIndex(fieldName)
    +
    +  override def dataType: DataType = {
    +    val structType = struct.dataType.asInstanceOf[StructType]
    +    val field = structType.fields(fieldIndex).copy(dataType = 
fieldValue.dataType)
    --- End diff --
    
    Nullability information should come from `fieldValue`.
    
    Also, I would suggest extending the constructor to allow for `Metadata` 
associated with the `fieldValue` expression, which can be added to the schema. 
It is not reasonable to assume that (a) any existing metadata should be 
preserved or (b) that there will be no need to associate new metadata.


---

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

Reply via email to