[comphead]

Which issue does this PR close?

Part of #5781, which stays open.

Rationale for this change

CometSize serializes its child twice, as CASE WHEN isnotnull(child) THEN size(child) ELSE <null answer> END. DataFusion's CASE evaluates the WHEN over the batch and then the THEN, so the child runs twice. That costs little for a column, but an expensive child pays twice. For a Scala UDF it is two dispatcher calls per row.

The native size already returns -1 for a NULL array or map, which is Spark's answer when spark.sql.legacy.sizeOfNull is on and ANSI is off, the default on Spark 3.x. A child that cannot be NULL never takes the NULL branch either. In both cases the child can go straight to size, the way CometArrayAppend already skips its guard for a non-nullable array.

This came up in a production Spark 3.4 job. Spark's InferFiltersFromGenerate added size(x) > 0 AND isnotnull(x) for an explode, and the plan shows that filter below an aggregate, with x replaced by the Scala UDF call that produced it. Comet called the UDF three times per row where Spark called it twice. Estimated from the UDF's own timers, the extra call and its JVM round trip were about a fifth of that stage's task time.

Evaluating the child once in these cases also keeps a nondeterministic child in step there (#5781). The guard stays for a nullable child when legacySizeOfNull is off, which includes Spark 4's default ANSI mode, so #5781 still applies to size in that case and to the other three expressions it lists.

The condition is the one @grorge123 proposed for CometSize in #5526. The discussion on #5781 left narrowing the guards for a follow-up, and this takes only the size part.

What changes are included in this PR?

• CometSize.convert emits the native size alone when legacySizeOfNull is on or the child is not nullable, and keeps the guard otherwise.
• Two native comments that said size only ever sees a null-free array in a Comet plan now say when it does.

How are these changes tested?

A new test in CometArrayExpressionSuite counts calls to a Scala UDF under size: with legacySizeOfNull on and a nullable UDF, and with it off and a non-nullable UDF. It checks the answer and native execution, and that Comet calls the UDF as many times as Spark does. With the guard, Comet would call it once more for each row whose array is not NULL. size.sql already covers both legacySizeOfNull settings for array and map columns with NULLs.

The new test has not been run locally yet. test-compile passes on the default profile and with -Pspark-3.5 -Pstrict-warnings.

[andygrove]

Thanks @comphead. This is a nice, contained change. Emitting the bare native size when legacySizeOfNull is on or the child cannot be NULL keeps the answers the same and stops a Scala UDF child from being called twice per row. The size.sql config matrix plus the new call-count test cover both the unguarded paths and the retained guard well.

One thing to tidy in CometSize. After the early return, createLiteralExprProto is only reached when legacySizeOfNull is false, so its if (legacySizeOfNull) -1 else null always gives null. Could the parameter go and the helper build a plain NULL IntegerType literal? That would stop it looking as if the guard can still answer -1.

[comphead]

Thanks @andygrove for the review. 01e0160 builds the NULL IntegerType literal in place and drops createLiteralExprProto, so the guard no longer looks as if it could answer -1. It also trims the comments in CometSize and size.rs.

[rich7420]

LGTM. Native size tests passed locally; current-head fork CI ran the call-count regression. Spark SQL checks were skipped.