[peterxcli]

Which issue does this PR close?

Closes #5436.

Rationale for this change

Allow top-level Variant scan output to use native columnar-to-row conversion when enabled.

What changes are included in this PR?

Preserve Variant field identity through JNI and write Spark's value-length/value/metadata layout into UnsafeRow memory. Nested Variant retains its existing fallback.

How are these changes tested?

variant.sql now requires CometNativeColumnarToRowExec for whole-value reads, scalar/container values, both null forms, siblings and shredded input. It also checks Spark C2R when native conversion is disabled. The operator assertion catches fallback even when results match.

Native layout and JVM tests cover exact UnsafeRow bytes, empty batches and buffer reuse. Updated the JNI integration for current upstream main.

[sunchao]

Summary

• Prior state and problem: Top-level Variant output required Spark columnar-to-row conversion because generic Struct encoding does not match Variant’s UnsafeRow layout.
• Design approach: Preserve Arrow field extension metadata through JNI and select a dedicated Variant writer.
• Correctness / compatibility analysis: The payload matches Spark’s UnsafeWriter across inspected Spark 4.0, 4.1 and 4.2 sources. SQL null and Variant null remain distinct. Spark 3 behavior and nested Variant fallback remain unchanged. No introduced P1/P2 issues found within this review.
• Key design decisions: Logical field identity distinguishes Variant from ordinary binary structs. The implementation fits the existing typed-array dispatch and reuses the conversion buffer. No additional per-row JNI calls are introduced. Performance was not benchmarked.
• Implementation sketch: Retain Field metadata, validate canonical binary children, write value length/value/metadata with aligned padding, and admit top-level Variant in the planner. The shared shuffle helper still supplies the same physical datatypes.
• Behavioral changes worth calling out: Relative to both the supplied base and branch-1.1, enabling native C2R now admits top-level Variant. Disabled conversion and nested Variant retain Spark fallback. SQL fixtures now assert the selected operator.
• Suggested improvements: No additional P1/P2-level changes identified.

Reviewed all 15 changed files at 88c90ddab1c41a252a4cb36ae0e6105a8c609fc7 against a4e72fd9d4e3bb8f459fc7950a98536724daf859. The PR is not draft. The snapshot and live discussion endpoints contained no existing reviews or comments.

Routed skills: review-comet-pr, review-comet-ffi-pr, and review-comet-shuffle-pr for the shared JNI helper.

Validation: All 22 exact-head conversion-module tests passed in a disposable harness using Arrow/Parquet 59.3.0 and a minimal error-type shim. A separate probe verified 44 native rows byte-for-byte against Spark 4.1.3’s writer and reader, covering sliced Arrow FFI input, buffer reuse, both null forms, and 3-/67-column schemas.

Exact-head CI remains pending: preflight, change detection, CodeQL, workflow analysis and labeling passed. Both Linux lint jobs are queued. Spark SQL, macOS and other optional suites are skipped. No native/JVM build-test verdict is available. Full Comet builds, Scala/SQL suites and Spark SQL shards were not run locally.

[rich7420]

Could we keep a convert_datatype_arrays wrapper? The new writeSortedPartitionsNative on main calls it, so merging would break compilation. writeSortedFileNative could use it too.

Suggested change

	fn convert_field_arrays(
	fn convert_datatype_arrays(
	env: &mut Env,
	serialized_datatypes: JObjectArray,
	) -> JNIResult<Vec<ArrowDataType>> {
	Ok(convert_field_arrays(env, serialized_datatypes)?
	.into_iter()
	.map(|field| field.data_type().clone())
	.collect())
	}
	fn convert_field_arrays(

[rich7420]

Could we also check that the scan stays native? Checking only ColumnarToRowExec would still pass if the Comet scan fell back.

[andygrove]

Thanks @peterxcli. The project now limits contributors who are not committers to three non-draft PRs at a time, and you had twelve open, so I am moving this one to draft. It is not a judgment on the change. I picked the three closest to merging, which are #5891, #5292 and #5048, and drafted the rest.

When one of those merges, please mark the next PR ready for review. The suggested order is: #6443, #6589, #6594, #5027, #5854, #5869, #5194, #5161, #6595. Drafts are still welcome to get review comments, rebases and CI runs, and you can keep pushing to this branch.

You have approval from sunchao on this one, so it is next in line once a slot opens. It conflicts with main now, so a rebase while it is in draft would help.