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

    https://github.com/apache/spark/pull/21598#discussion_r197618603
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
 ---
    @@ -75,28 +75,47 @@ trait BinaryArrayExpressionWithImplicitCast extends 
BinaryExpression
           > SELECT _FUNC_(array('b', 'd', 'c', 'a'));
            4
       """)
    -case class Size(child: Expression) extends UnaryExpression with 
ExpectsInputTypes {
    +case class Size(
    +    child: Expression,
    +    legacySizeOfNull: Boolean)
    +  extends UnaryExpression with ExpectsInputTypes {
    +
    +  def this(child: Expression) =
    +    this(
    +      child,
    +      legacySizeOfNull = SQLConf.get.getConf(SQLConf.LEGACY_SIZE_OF_NULL))
    +
       override def dataType: DataType = IntegerType
       override def inputTypes: Seq[AbstractDataType] = 
Seq(TypeCollection(ArrayType, MapType))
    -  override def nullable: Boolean = false
    +  override def nullable: Boolean = if (legacySizeOfNull) false else 
super.nullable
     
       override def eval(input: InternalRow): Any = {
         val value = child.eval(input)
         if (value == null) {
    -      -1
    +      if (legacySizeOfNull) -1 else null
         } else child.dataType match {
           case _: ArrayType => value.asInstanceOf[ArrayData].numElements()
           case _: MapType => value.asInstanceOf[MapData].numElements()
    +      case other => throw new UnsupportedOperationException(
    +        s"The size function doesn't support the operand type 
${other.getClass.getCanonicalName}")
         }
       }
     
       override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
    -    val childGen = child.genCode(ctx)
    -    ev.copy(code = code"""
    +    if (legacySizeOfNull) {
    +      val childGen = child.genCode(ctx)
    +      ev.copy(code = code"""
           boolean ${ev.isNull} = false;
           ${childGen.code}
           ${CodeGenerator.javaType(dataType)} ${ev.value} = ${childGen.isNull} 
? -1 :
             (${childGen.value}).numElements();""", isNull = FalseLiteral)
    +    } else {
    +      child.dataType match {
    +        case _: ArrayType | _: MapType => defineCodeGen(ctx, ev, c => 
s"($c).numElements()")
    +        case other => throw new UnsupportedOperationException(
    --- End diff --
    
    Could you reorganize the code? The current flow looks confusing. It appears 
like that the other types are not supported when `legacySizeOfNull` is false. 
However, the input types are already limited to ArrayType and MapType. 


---

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

Reply via email to