[SPARK-45078][SQL] Fix array_insert ImplicitCastInputTypes not work

[Hisoka-X]

What changes were proposed in this pull request?

This PR fix call array_insert with different type between array and insert column, will throw exception. Sometimes it should be execute successed.
eg:

select array_insert(array(1), 2, cast(2 as tinyint))

The ImplicitCastInputTypes in ArrayInsert always return empty array at now. So that Spark can not convert tinyint to int.

Why are the changes needed?

Fix error behavior in array_insert

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add new test.

Was this patch authored or co-authored using generative AI tooling?

No

[Hisoka-X]

cc @cloud-fan @dongjoon-hyun @Daniel-Davies

[Daniel-Davies]

Thank you for fixing this @Hisoka-X!

[MaxGekk]

Can't we require if someone extends ExpectsInputTypes then inputTypes shall return a non-empty Seq?

[Hisoka-X]

Can't we require if someone extends ExpectsInputTypes then inputTypes shall return a non-empty Seq?

Maybe because empty means do nothing this time?

[MaxGekk]

Maybe because empty means do nothing this time?

If do nothing, what's the reason to expect some input types which extending of ExpectsInputTypes assumes?

[Hisoka-X]

Maybe because empty means do nothing this time?

If do nothing, what's the reason to expect some input types which extending of ExpectsInputTypes assumes?

Different input types may require different implicit conversions. Whether to perform conversion depends on the type of input and the characteristics of the function. There is no way to know whether conversion is required when the function is defined.

You mean we should do like this? @MaxGekk

  override def inputTypes: Seq[AbstractDataType] = {
    (srcArrayExpr.dataType, posExpr.dataType, itemExpr.dataType) match {
      case (ArrayType(e1, hasNull), e2: IntegralType, e3) if (e2 != LongType) =>
        TypeCoercion.findTightestCommonType(e1, e3) match {
          case Some(dt) => Seq(ArrayType(dt, hasNull), IntegerType, dt)
          case _ => Seq(ArrayType(e1), IntegerType, e1)
        }
      case (e1, e2, e3) => Seq(e1, e2, e1)
    }
  }

[MaxGekk]

Maybe because empty means do nothing this time?
You mean we should do like this? @MaxGekk

Yep, eliminate the special meaning of empty Seq.

[MaxGekk]

In any case, this PR LGTM

[Hisoka-X]

The collectionOperations.scala have a lots of Seq.empty. If we need remove it all, I can create a PR for it.

[MaxGekk]

The collectionOperations.scala have a lots of Seq.empty. If we need remove it all, I can create a PR for it.

Let's leave them as is so far.

[MaxGekk]

+1, LGTM. Merging to master/3.5/3.4.
Thank you, @Hisoka-X.

[MaxGekk]

@Hisoka-X The changes cause some conflicts in 3.4. Could you open a PR with backport to branch-3.4, please.

[cloud-fan]

what a mistake...