refactor: share one helper for pushing a struct's null mask into its children

[viirya]

Which issue does this PR close?

Closes #5768.

Rationale for this change

Three places implemented the same Spark rule — a field of a null struct is null, so the
parent's nulls have to reach the children before the children are read — and each did it
slightly differently:

	union when
GetStructField::project_field	a null buffer is present, via the checked ArrayData builder
the hash kernels' struct branch	null_count() > 0, via StructArray::flatten
get_array_struct_fields	the parent and child null counts differ

The issue was filed as a refactor, on the grounds that all three were correct and the risk
was drift. That turned out to be half right: the third guard is wrong. Equal null counts
do not imply the nulls sit at the same rows, so whenever the child had a null of its own at a
different row the counts could match and the parent's null was dropped. There is a test for
exactly that shape, which a count-based check fails.

What changes are included in this PR?

datafusion_comet_common::struct_nulls, with child_with_parent_nulls for one field and
children_with_parent_nulls for all of them, and the three call sites switched over. Net -41
lines at the call sites.

The three decisions the issue asked about:

• Shape. Both, with the plural built on the singular, since project_field wants one
  field and the hash path wants all of them.
• Checked or unchecked. Unchecked, as flatten already did. The union only adds nulls, so
  every data buffer is carried over unchanged and revalidating them is wasted work. This
  matters asymmetrically: project_field runs once per batch in evaluate and did not care
  either way, while the hash path reaches this once per element of an array<struct<..>>,
  where re-checking a string child means rescanning its whole UTF-8 values buffer. Unifying on
  checked would have pushed the batch-level caller's cost onto the element-level one.
• The null_count() > 0 guard. Applied to all three. It skips the union when the parent
  has no null to contribute, which covers both no buffer at all and a present-but-all-valid
  buffer — the latter is what slicing leaves behind. NullBuffer stores its null count, so the
  test is O(1).

It lives in its own module rather than in schema.rs, which is about reconciling declared and
runtime types rather than null masks.

How are these changes tested?

Six unit tests on the helper: a hidden child value under a null parent, no parent null buffer
(asserting the same array comes back rather than a rebuilt one), a present-but-all-valid
buffer, a child's own null surviving the union, an empty struct, and the equal-counts-different-rows
case that pins the guard fix.

Existing coverage for the three call sites passes unchanged: cargo test gives 720 + 5 in
spark-expr and 40 in common, and on the Spark side CometHashExpressionSuite (40),
CometArrayExpressionSuite (59) and CometExpressionSuite (141). cargo fmt --check and
clippy -D warnings are clean.

Additional context

Raised by @andygrove reviewing #5754. The two earlier fixes in this area were #4432
(GetStructField) and #5754 (the hash kernels); this unifies them with the third site, which
the issue had not accounted for.

[sunchao]

Correctness

Reviewed 41e36bb0 against bb9e7402. I found no new P1/P2 issue in the five-file change. This consolidates three implementations of struct-parent validity propagation and fixes the array-field caller's equal-null-count shortcut. Parent and child null counts can be equal while their nulls occupy different rows. The shared helper intersects validity whenever the parent actually has nulls, preserving the child's existing nulls and hiding values under null struct rows.

That behavior matches the maintained Spark 3.5 and 4.0 implementations: struct extraction returns null for a null parent, array-of-struct extraction returns null for either a null element or a null selected field, and hashing a null value preserves the incoming seed. GetStructField retains its parent-or-field nullability rule. List/LargeList extraction retains the outer offsets and validity. The hash path performs the same child-array operation as Arrow 59.3.0 StructArray::flatten, with recursive struct/list/map handling and dictionary seed handling unchanged. Dictionary parent nulls belong on the keys, while null dictionary values remain represented in the shared value array.

The current Rust CI job passes workspace Clippy with warnings denied and reports 1,199 tests passed, with four skipped. It explicitly passes all six new helper tests and the existing dictionary, null-struct and null-struct-list regressions for both hashers. Its checkout is merge f8d76a7a, whose raw parents are this exact B/H pair and whose entire tree equals H. At the September 8, 08:30:53 UTC refresh, 20 checks passed, seven were skipped, 30 were running and one was queued, with no failed checks. I ran no local product tests. The PR body's JVM suite totals remain author-reported results, and maintained Spark 3.4/4.1 source branches were unavailable for comparison.

Performance

The no-parent-null and all-valid-parent paths clone the existing child Arc directly. The all-children API retains one vector of child references, matching the old hash fast path. With actual parent nulls, the helper changes validity and array wrappers while sharing payload buffers. It does not decode dictionary values or expand list/map masks. Replacing flatten also avoids constructing field metadata that the hash caller immediately discarded, and the singular API touches only the selected child.

The parent null-count test is cached and constant time. Its repetition inside the plural helper adds no data scan. Where both validity buffers exist, their intersection remains proportional to the logical row count. No new material allocation or traversal regression was established. These are source-level cost observations, with no measured component or query-level speedup claimed.

Design

A common helper is appropriate because expression projection and hashing need the same parent-mask operation but consume different numbers of children. Keeping both a single-child and all-children entry point avoids forcing projection to process unused siblings. The implementation leaves field metadata with the caller, which is suitable for hashing because it only consumes arrays. The documented unchecked reconstruction follows the existing Arrow flatten path for the bitmap-bearing representations used here, preserving payload and child data while making validity more restrictive.

The issue discussion also mentions timestamp conversion and nested casts. Neither becomes a caller in this PR, and the timestamp converter is byte-identical at B/H. Keeping those separate avoids conflating field projection with recursive conversion visibility and validity restoration.

Abstraction & complexity

The small common module removes divergent guards and gives the equal-count regression one central place to live. Its six focused tests cover hidden parent values, existing child nulls, both no-null representations, distinct null positions with equal counts and an empty struct. Existing native hash tests cover the nested and dictionary callers. The new tests do not independently establish every Arrow representation or a new end-to-end SQL case for equal-count positions, so the compatibility assessment also relies on the caller and dependency source checks above. No additional abstraction or change is requested.

[peterxcli]

Could move import to top

use datafusion_comet_common::child_with_parent_nulls;

Suggested change

	let columns = datafusion_comet_common::children_with_parent_nulls(struct_array)?;
	let columns = children_with_parent_nulls(struct_array)?;