fix: compare greatest and least struct arguments by position

[viirya]

Which issue does this PR close?

Closes #6758.

Rationale for this change

With spark.sql.caseSensitive=false, Spark accepts greatest/least arguments whose struct field names differ only in case, and compares them field by field by position. Comet passed the arguments through DataFusion's coercion, whose struct coercion matches fields by name when both structs hold the same set of names. Arrow's struct cast then reorders the values by name, so Comet compared different pairs of fields and returned a different row from Spark.

What changes are included in this PR?

• Adds coerce_to_common_type next to if_common_type and coerce_branch in conditional_funcs/case_when.rs, built from them. It folds if_common_type over the arguments to get a positional common type that keeps the first argument's field names and merges nullability, then casts each argument with Comet's Spark Cast, whose struct cast is positional.
• The planner calls it for greatest and least before DataFusion's coercion runs. The arguments then share one type, so DataFusion's coercion has nothing left to reorder.

This reuses the same helpers that #6428 builds on for comparisons and CASE, rather than adding another copy of the positional logic.

How are these changes tested?

• New SQL file expressions/math/greatest_least_struct_field_case.sql, run with spark.sql.caseSensitive=false. It covers greatest and least with float and integer fields, rows where the positional and by-name answers differ, three arguments, structs nested in arrays, and arguments that also differ in nested nullability. All 8 queries fail without this change: 7 return different rows from Spark, and the three-argument query fails with Cannot cast nullable struct field 'X' to non-nullable field.
• The existing Rust tests if_reconciles_case_variant_fields_positionally and if_reconciles_struct_field_nullability_and_names cover the helpers this reuses, on swapped case-variant names and merged nullability.
• The existing greatest, least, float, and conditional SQL files still pass.

This pull request and its description were written by Isaac.

[sunchao]

Reviewed the full four-file diff from b56349697b786ff2ad1c1bcf6ecf45b809af5f30 to 6e1dd91890832a114ccc55c0f1de60a48106c064. The PR is not a draft. No introduced P1/P2 issues found within this review.

Read AGENTS.md and used review-comet-pr and review-comet-expression-pr, with the expression, SQL-testing, development, CI, and timezone guidance. The discussion snapshot and live review/comment checks contained no existing feedback or unresolved blockers.

Summary

• Prior state and problem: DataFusion could reconcile struct arguments by field name, reordering values that Spark compares by position. With case-insensitive analysis, this produced incorrect greatest/least results or a nullable-field cast error.
• Design approach: Normalize arguments to a positional common Arrow type before DataFusion resolves the function and applies its coercion.
• Correctness: Checked Spark’s Greatest, Least, ComplexTypeMergingExpression, type-coercion helpers, ordering implementation, and expression tests. Retaining the first argument’s names and merging nested nullability matches Spark. Existing comparison kernels retain responsibility for null skipping, NaN ordering, and first-argument tie behavior. No P1/P2 correctness issue identified.
• Compatibility analysis: Compared the relevant Spark sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0. Their positional comparison and type-merging contracts agree. Spark still validates argument compatibility before native planning. No configuration, support-level, or serialization contract changes are introduced.
• Key design decisions: Fold the existing if_common_type over all arguments, preserve positional field order, merge nullability recursively, and avoid wrapping arguments whose types already match. Retaining the existing path when no common type is available avoids introducing another type-admission policy.
• Implementation sketch: Export coerce_to_common_type, call it for greatest and least in the native planner, and validate it with a Rust unit test and eight SQL queries covering integer/float structs, three arguments, arrays of structs, and nested nullability.
• Performance: The additional work is a planning-time traversal of argument types. Matching arguments receive no runtime cast. Structural relabeling uses existing casts that preserve underlying value buffers where leaf types match. No evidence-backed performance regression identified. No benchmark was run.
• Design: The intervention is localized at the point before incompatible name-based coercion occurs. It preserves the existing function implementations and avoids duplicating comparison logic.
• Abstraction & complexity: Reusing if_common_type and coerce_branch keeps positional reconciliation in one implementation. The small exported argument-list helper is appropriately scoped. Existing IF and CASE behavior is unchanged.
• Behavioral changes worth calling out: Compared the touched planner path with latest release branch branch-1.1 at e9efd9f764ee0a59b7898ff028d6985d4a7a28e1. This intentionally fixes the same name-based coercion path present there. For the reported id = 0 example, positional comparison selects [1.0, 0.0] rather than [0.0, 1.0]. No unintended release-relative regression identified.
• Suggested improvements: No additional change meeting the P1/P2 reporting bar was identified.

Exact-head CI: All 71 checks completed, with 40 successful and 31 skipped. Both required-check gates passed. Linux Rust, expression/operator suites, TPC checks, and all nine labeled Spark 4.1 SQL shards passed. See runs 37680371798 and 37680393333.

Validation: The local Rust test coerce_to_common_type_is_positional passed. A focused Spark 4.1.3 Maven run passed six test executions across four SQL files: greatest, least, greatest_least_floating_point, and greatest_least_struct_field_case. The native library was byte-verified against the exact-head CI artifact. The working tree remains clean.

Validation limits: Other Spark versions were checked against source, but their runtime suites were not run locally and were skipped in this PR’s CI. macOS runtime coverage was also skipped. No performance measurements were taken.

[comphead]

Thanks @viirya. Aligning the arguments in the planner before DataFusion's coercion looks like the right place for this. The inline comments cover the overlap with #6428, a unit test that repeats existing coverage, and two inert configs in the SQL file.

[comphead]

#6428 at its latest head (ba8f2a5cd4) renames if_common_type and coerce_branch to positional_common_type, which takes a PositionalTypeCoercion mode, and a public cast_to_common_type. Its new IN arm in the planner also folds a common type over the operands and casts each one, as this function does. The two PRs conflict in planner.rs, case_when.rs and mod.rs, so whichever lands second has to adapt. Could that one leave a single helper for IN, greatest and least? MetadataOnly looks like the right mode here, since #6428 documents it as the comparison mode where Catalyst has already coerced the leaf types.

[viirya]

Agreed. #6428 is still open, so this PR keeps its own helper for now. Whichever lands second will fold positional_common_type(.., PositionalTypeCoercion::MetadataOnly) over the arguments and cast each one with cast_to_common_type, dropping coerce_to_common_type. That leaves one path for IN, greatest and least. If #6428 lands first, I'll rebase this PR onto it and make that change here.

[comphead]

if_reconciles_case_variant_fields_positionally and if_reconciles_struct_field_nullability_and_names already pin if_common_type and coerce_branch on swapped case-variant names and merged nullability. The new SQL file covers this function end to end for structs and arrays of structs, including the three-argument fold that this test does not reach, and the description says all eight queries fail without the fix. Could this test be dropped, since it calls the same helpers with two arguments?

[viirya]

Dropped in e1d88f2.

[comphead]

With spark.comet.exec.range.enabled=true, CometExecRule turns range(...) into CometRangeExec and only falls back to the Spark-to-Arrow conversion when that operator declines a range (around line 504 of CometExecRule.scala). So the two sparkToColumnar lines have no effect here, and naming Range in spark.comet.sparkToColumnar.supportedOperatorList is deprecated per its doc in CometConf.scala. Could we drop both lines? The file would then fail if native Range ever stopped taking these ranges, rather than quietly running on a converted leaf.

[viirya]

Dropped both lines in e1d88f2. Every query in the file still runs natively through CometRangeExec.