[SPARK-26965][SQL] Makes ElementAt nullability more precise for array cases

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

@@ -1929,7 +1929,8 @@ case class ArrayPosition(left: Expression, right: Expression)
        b
   """,
   since = "2.4.0")
-case class ElementAt(left: Expression, right: Expression) extends GetMapValueUtil {
+case class ElementAt(left: Expression, right: Expression)
+    extends GetMapValueUtil with GetArrayItemUtil {
 
   @transient private lazy val mapKeyType = left.dataType.asInstanceOf[MapType].keyType
 
@@ -1974,7 +1975,10 @@ case class ElementAt(left: Expression, right: Expression) extends GetMapValueUti
     }
   }
 
-  override def nullable: Boolean = true
+  override def nullable: Boolean = left.dataType match {
+    case _: ArrayType => computeNullabilityFromArray
+    case _: MapType => computeNullabilityFromMap
+  }
 
   override def nullSafeEval(value: Any, ordinal: Any): Any = doElementAt(value, ordinal)
 

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala

@@ -216,24 +216,15 @@ case class GetArrayStructFields(
 }
 
 /**
- * Returns the field at `ordinal` in the Array `child`.
- *
- * We need to do type checking here as `ordinal` expression maybe unresolved.
+ * Common trait for [[GetArrayItem]] and [[ElementAt]].
  */
-case class GetArrayItem(child: Expression, ordinal: Expression)
-  extends BinaryExpression with ExpectsInputTypes with ExtractValue with NullIntolerant {
-
-  // We have done type checking for child in `ExtractValue`, so only need to check the `ordinal`.
-  override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, IntegralType)
-
-  override def toString: String = s"$child[$ordinal]"
-  override def sql: String = s"${child.sql}[${ordinal.sql}]"
+trait GetArrayItemUtil extends BinaryExpression {
 
-  override def left: Expression = child
-  override def right: Expression = ordinal
+  private val child = left
+  private val ordinal = right
 
   /** `Null` is returned for invalid ordinals. */
-  override def nullable: Boolean = if (ordinal.foldable && !ordinal.nullable) {
+  protected def computeNullabilityFromArray: Boolean = if (ordinal.foldable && !ordinal.nullable) {
     val intOrdinal = ordinal.eval().asInstanceOf[Number].intValue()
     child match {
       case CreateArray(ar) if intOrdinal < ar.length =>
@@ -247,7 +238,25 @@ case class GetArrayItem(child: Expression, ordinal: Expression)
   } else {
     true
   }
+}
 
+/**
+ * Returns the field at `ordinal` in the Array `child`.
+ *
+ * We need to do type checking here as `ordinal` expression maybe unresolved.
+ */
+case class GetArrayItem(child: Expression, ordinal: Expression)
+  extends GetArrayItemUtil with ExpectsInputTypes with ExtractValue with NullIntolerant {
+
+  // We have done type checking for child in `ExtractValue`, so only need to check the `ordinal`.
+  override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, IntegralType)
+
+  override def toString: String = s"$child[$ordinal]"
+  override def sql: String = s"${child.sql}[${ordinal.sql}]"
+
+  override def left: Expression = child
+  override def right: Expression = ordinal
+  override def nullable: Boolean = computeNullabilityFromArray
   override def dataType: DataType = child.dataType.asInstanceOf[ArrayType].elementType
 
   protected override def nullSafeEval(value: Any, ordinal: Any): Any = {
@@ -281,10 +290,29 @@ case class GetArrayItem(child: Expression, ordinal: Expression)
 }
 
 /**
- * Common base class for [[GetMapValue]] and [[ElementAt]].
+ * Common trait for [[GetMapValue]] and [[ElementAt]].
  */
+trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
+
+  private val child = left
+  private val key = right
+
+  /** `Null` is returned for invalid ordinals. */
+  protected def computeNullabilityFromMap: Boolean = if (key.foldable && !key.nullable) {
+    val keyObj = key.eval()
+    child match {
+      case m: CreateMap if m.resolved =>
+        m.keys.zip(m.values).filter { case (k, _) => k.foldable && !k.nullable }.find {
+          case (k, _) if k.eval() == keyObj => true
+          case _ => false
+        }.map(_._2.nullable).getOrElse(true)
+      case _ =>
+        true
+    }
+  } else {
+    true
+  }
 
-abstract class GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
   // todo: current search is O(n), improve it.
   def getValueEval(value: Any, ordinal: Any, keyType: DataType, ordering: Ordering[Any]): Any = {
     val map = value.asInstanceOf[MapData]
@@ -379,24 +407,7 @@ case class GetMapValue(child: Expression, key: Expression)
 
   override def left: Expression = child
   override def right: Expression = key
-
-  /** `Null` is returned for invalid ordinals. */
-  override def nullable: Boolean = if (key.foldable && !key.nullable) {
-    val keyObj = key.eval()
-    child match {
-      case m: CreateMap if m.resolved =>
-        m.keys.zip(m.values).filter { case (k, _) => k.foldable && !k.nullable }.find {
-          case (k, _) if k.eval() == keyObj => true
-          case _ => false
-        }.map(_._2.nullable).getOrElse(true)
-      case _ =>
-        true
-    }
-  } else {
-    true
-  }
-
-
+  override def nullable: Boolean = computeNullabilityFromMap
   override def dataType: DataType = child.dataType.asInstanceOf[MapType].valueType
 
   // todo: current search is O(n), improve it.

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala

@@ -1092,6 +1092,58 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
     checkEvaluation(ElementAt(mb0, Literal(Array[Byte](3, 4))), null)
   }
 
+  test("SPARK-26965 correctly handles ElementAt nullability for arrays") {
+    // CreateArray case
+    val a = AttributeReference("a", IntegerType, nullable = false)()
+    val b = AttributeReference("b", IntegerType, nullable = true)()
+    val array = CreateArray(a :: b :: Nil)
+    assert(!ElementAt(array, Literal(0)).nullable)
+    assert(ElementAt(array, Literal(1)).nullable)
+    assert(!ElementAt(array, Subtract(Literal(2), Literal(2))).nullable)
+    assert(ElementAt(array, AttributeReference("ordinal", IntegerType)()).nullable)
+
+    // GetArrayStructFields case
+    val f1 = StructField("a", IntegerType, nullable = false)
+    val f2 = StructField("b", IntegerType, nullable = true)
+    val structType = StructType(f1 :: f2 :: Nil)
+    val c = AttributeReference("c", structType, nullable = false)()
+    val inputArray1 = CreateArray(c :: Nil)
+    val inputArray1ContainsNull = c.nullable
+    val stArray1 = GetArrayStructFields(inputArray1, f1, 0, 2, inputArray1ContainsNull)
+    assert(!ElementAt(stArray1, Literal(0)).nullable)
+    val stArray2 = GetArrayStructFields(inputArray1, f2, 1, 2, inputArray1ContainsNull)
+    assert(ElementAt(stArray2, Literal(0)).nullable)
+
+    val d = AttributeReference("d", structType, nullable = true)()
+    val inputArray2 = CreateArray(c :: d :: Nil)
+    val inputArray2ContainsNull = c.nullable || d.nullable
+    val stArray3 = GetArrayStructFields(inputArray2, f1, 0, 2, inputArray2ContainsNull)
+    assert(!ElementAt(stArray3, Literal(0)).nullable)
+    assert(ElementAt(stArray3, Literal(1)).nullable)
+    val stArray4 = GetArrayStructFields(inputArray2, f2, 1, 2, inputArray2ContainsNull)
+    assert(ElementAt(stArray4, Literal(0)).nullable)
+    assert(ElementAt(stArray4, Literal(1)).nullable)
+  }
+
+  test("SPARK-26965 correctly handles ElementAt nullability for maps") {
+    // String key test
+    val k1 = Literal("k1")
+    val v1 = AttributeReference("v1", StringType, nullable = true)()
+    val k2 = Literal("k2")
+    val v2 = AttributeReference("v2", StringType, nullable = false)()
+    val map1 = CreateMap(k1 :: v1 :: k2 :: v2 :: Nil)
+    assert(ElementAt(map1, Literal("k1")).nullable)
+    assert(!ElementAt(map1, Literal("k2")).nullable)
+    assert(ElementAt(map1, Literal("non-existent-key")).nullable)
+
+    // Complex type key test
+    val k3 = Literal.create((1, "a"))
+    val k4 = Literal.create((2, "b"))
+    val map2 = CreateMap(k3 :: v1 :: k4 :: v2 :: Nil)
+    assert(ElementAt(map2, Literal.create((1, "a"))).nullable)
+    assert(!ElementAt(map2, Literal.create((2, "b"))).nullable)
+  }
+
   test("Concat") {
     // Primitive-type elements
     val ai0 = Literal.create(Seq(1, 2, 3), ArrayType(IntegerType, containsNull = false))