Skip to content

Follow-ups to native NULL short-circuiting of array and map functions #6850

Description

@andygrove

What is the problem the feature request solves?

#6716, which fixes #6613, makes native array and map functions skip their later arguments on rows where the array or map is NULL, as Spark's null-intolerant BinaryExpressions and TernaryExpressions do. It sets null_short_circuit on ScalarFunc and ListExtract. A few related shapes are left over:

  • element_at still uses CometElementAt's CASE WHEN <operand> IS NOT NULL guard (feat: address remaining issues for CreateArray #5766). The guard evaluates the operand twice, and under ANSI it declines a nullable nondeterministic operand. Setting the flag on its ListExtract would evaluate the operand once and let that decline go. This overlaps fix: dispatch map lookups with normalized keys and nondeterministic null-guarded children #5867.
  • map_from_arrays nests CASE WHEN keys IS NOT NULL and CASE WHEN values IS NOT NULL, so it serializes and evaluates both arrays twice. With the flag, each would be evaluated once. fix: enforce null-key rejection and mapKeyDedupPolicy in native map construction #5854 rewrites this serde, so this should come after it.
  • array_contains on flat float arrays (feat: run array_contains on float elements natively with Spark's equa… #6599) keeps a nullable array whose value is neither a column nor a literal on the codegen dispatcher, through eagerEvalReason in CometArrayContains. Now that spark_array_contains gets the short-circuit, that decline can go.
  • Shared arguments where Spark doesn't eliminate subexpressions. canShortCircuitNulls doesn't skip an argument that holds a subexpression the operator's expressions share, because Spark's generated code evaluates those for every row and raises if one fails. With spark.sql.subexpressionElimination.enabled=false, Spark skips the argument instead, so Comet raises where Spark returns NULL.
  • array_repeat is the other way around: Spark's ArrayRepeat.eval reads the count before the element, so a left-to-right guard doesn't fit it. With whole-stage codegen, which is the default, Spark's generated code evaluates the element first and raises the same way Comet does. On 3.4.3, SELECT array_repeat(CAST(s AS INT), cnt) over a row ('bad', NULL) raises CAST_INVALID_INPUT in both, so nothing differs in the default configuration.

Describe the potential solution

Move element_at, and map_from_arrays after #5854, to withNullShortCircuit. Drop CometArrayContains.eagerEvalReason. Decide whether the shared-argument case should follow spark.sql.subexpressionElimination.enabled, accepting that Spark's aggregate update path eliminates subexpressions whatever that flag says.

Additional context

Raised in the #6716 review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions