fix: preserve Arrow Field metadata across C Data exports

[peterxcli]

Which issue does this PR close?

Closes #5547.

This is the first atomic change in #5546, extracted from #5407. It is intentionally independent of Variant scan admission.

Rationale for this change

Native output currently exports each Arrow array with an FFI_ArrowSchema reconstructed from ArrayData.data_type(). A DataType describes the physical layout, but it does not carry the enclosing Field name, nullability, or metadata. Logical types identified by Field metadata therefore lose their identity at the Arrow C Data boundary.

The RecordBatch already owns the authoritative Field for each output column. Exporting that Field fixes the shared boundary once for query output and the native Parquet reader.

ArrowSchema names are NUL-terminated C strings. Passing the Field also makes embedded NUL names visible to the exporter, so this change substitutes U+FFFD only in the exported name while retaining the Field datatype, nullability, and metadata. Spark's logical output name remains owned by its plan.

What changes are included in this PR?

• Pass the corresponding RecordBatch Field to SparkArrowConvert::move_to_spark.
• Construct FFI_ArrowSchema from the Field while leaving FFI_ArrowArray export unchanged.
• Use the original output Field for offset-normalized arrays.
• Apply the same Field-aware export in Native.currentColumnBatch.
• Add a focused round-trip test for name sanitization, datatype, nullability, and metadata.

This PR does not add a Variant protobuf type, Arrow extension mapping, Parquet normalization, or scan admission. Those remain in the later #5546 subtasks.

How are these changes tested?

cd native
cargo fmt --all -- --check
cargo test -p datafusion-comet test_ffi_schema_preserves_field_and_sanitizes_nul_name
cd ..
git diff --check upstream/main...HEAD

The focused Rust test passed: 1 passed, 0 failed, 199 filtered out. Building that test also compiled both updated native export callers.

[peterxcli]

cc @sunchao

[sunchao]

Completed five independent review passes covering native FFI correctness, Spark imports/planning, ownership and error handling, compatibility, and test coverage at a0c8f89.

Found one P2 regression in broadcast coalescing, detailed inline. The Spark 4.1.3/JDK 17 reproduction fails on this head and passes on the exact base 98cd8c9. The focused Rust test, all four NativeUtilSuite tests, and standalone native/JNI compatibility probes pass. The full Spark version/CI matrix was not run locally.

[sunchao]

Re-reviewed 7ef3bbd across five independent scopes. No new actionable findings.

The earlier broadcast-nullability regression is fixed. All 38 JVM tests passed on Spark 4.1.3/JDK 17: the full CometJoinSuite (30), NativeUtilSuite (4), and four independent UNION/BROADCAST cases with AQE on/off and actual null values. The existing large homogeneous-broadcast test still confirms successful coalescing.

The committed regression also passed a sensitivity check: removing only the guard restores the schema-mismatch failure; simulating the old primitive export schema fails its zero-coalescing-metrics assertion. Additional FFI, buffer reuse, and cleanup probes passed.

Native sources are unchanged from the previously reviewed head; I verified that identity and reused the tested native library. The full Spark version/CI matrix was not run locally.

[sunchao]

The previously reported broadcast issue is fixed. Re-review and focused validation at 7ef3bbd found no remaining actionable issues. CI is still running.

[andygrove]

Thanks for splitting this out of #5407, the atomic scope makes it much easier to reason about. Hoisting FFI_ArrowArray::new and the schema conversion above the alignment branch is a real improvement on its own. The old code could write the array through array_ptr and then fail on the schema conversion, leaving the JVM holding a half-initialized pair. Now both are built before either write.

I also checked that RecordBatch::try_new validates column data types against the schema and rejects nulls in non-nullable fields, so sourcing the exported type from the schema field rather than ArrayData::data_type() cannot introduce a schema/buffer disagreement. That part is safe.

My main question is about scope, left inline on ffi_schema_for_field. The rest are smaller points on the coalescing guard, the Parquet path, and test coverage.

[sunchao]

[P2] Keep broadcast coalescing safe when array field metadata differs

At 754cc2f, two native V1 Parquet scans of (k INT, v ARRAY<INT>) with explicit read schemas carrying different top-level parquet.field.id values on v can fail when their UNION ALL is the broadcast side of an inner join on k and v remains in the result. The list element fields can be identical. The metadata export and IPC paths preserve each array field's PARQUET:field_id; CometUnionExec forwards the child batches without normalizing those fields.

Arrow Java 18.3.0's outer VectorSchemaRootAppender check ignores metadata, but its ListVector append path uses TypeEqualsVisitor(targetVector, false, true). The differing metadata therefore throws IllegalArgumentException and aborts the broadcast. BASE omitted these outer IDs; the previous reviewed head retained them but returned the original buffers when schemas differed. Please retain a compatible fallback or reconcile safe metadata differences while preserving the intended metadata export.

This is a source-derived finding for small, nonempty, non-dictionary batches, with Parquet columns read by name, native scan/execution/union/broadcast enabled, and AQE disabled. I did not execute the query. Primitive-only metadata differences do not trigger this list-specific check.

[sunchao]

Re-reviewed ef4b5993. The new fallback addresses the P2 array-metadata coalescing failure reported in the previous review.

I’m withholding approval because the macOS scan job ended in a native-thread SIGBUS and its relationship to this change remains unresolved.

[sunchao]

Re-reviewed ef4b599373c5f9002b142cd9bb7d526d46d40d95 with five independent scopes using the repository's review checklist. No new actionable findings. The metadata-only export keeps the previous top-level names and nullability. Ordinary unions coalesce again, and incompatible array metadata safely falls back to the original buffers.

Fresh local validation passed: 40 JVM tests on Spark 4.1.3/JDK 17, including the full CometJoinSuite, NativeUtilSuite, four additional array/null/AQE cases, and the field-ID metadata assertion. That assertion fails with Set(null) using the exact-base native library and passes with this head, confirming that it protects the native-to-JVM metadata boundary. The focused Rust test and independent native, JNI/IPC, and coalescing component probes also passed. I did not run the full Spark/platform matrix locally.

CI currently has 60 passing, 9 skipped, and 2 failed checks. The Linux exec job failed before its tests when Maven Central returned HTTP 429 for a formatting dependency.

I also investigated the macOS scan crash that prompted the earlier concern. The crash dump and that run's exact native artifact resolve the return address to hdfsThreadDestructor + 80, immediately after its indirect GetJavaVM call. This strongly matches the existing HDFS teardown failure in #5023. The exact base and previous head both passed the matching macOS scan suite, and the previous head has identical native sources. I found no new PR defect behind these failures. Please rerun the two failed jobs before merging.

[sunchao]

@peterxcli can you check the CI failure and see whether it is related? I've retried a few times

Error: Unable to download artifact(s): Artifact not found for name: native-lib-macos
        Please ensure that your artifact is not expired and the artifact was uploaded using a compatible version of toolkit/upload-artifact.
        For more information, visit the GitHub Artifacts FAQ: https://github.com/actions/toolkit/blob/main/packages/artifact/docs/faq.md

[peterxcli]

@peterxcli can you check the CI failure and see whether it is related? I've retried a few times

Error: Unable to download artifact(s): Artifact not found for name: native-lib-macos
        Please ensure that your artifact is not expired and the artifact was uploaded using a compatible version of toolkit/upload-artifact.
        For more information, visit the GitHub Artifacts FAQ: https://github.com/actions/toolkit/blob/main/packages/artifact/docs/faq.md

@sunchao Seems like it's a flaky? I merge upstream main again and this error is gone, however it looks like other flaky failure just appeared.

[peterxcli]

@sunchao I think remaining failure are all intermittent, do you think this is ok to merge? Thanks!

[sunchao]

@peterxcli From the code-review side, my approval still stands for 979cd3ae06b13c3037eab379f3bf9343ce3fed7a, with no new P1/P2 findings in the current version. I can't confirm that the remaining failure is intermittent, though. The current macOS scan job still has a native SIGBUS. The diagnostics identify the HDFS thread-cleanup location, but do not by themselves prove a flake or rule out a PR interaction. I have not rerun that job, so I would keep the unresolved CI result separate from my code approval.

[peterxcli]

@sunchao looks like all test pass now

[sunchao]

@peterxcli Confirmed: the current checks for 979cd3ae06b13c3037eab379f3bf9343ce3fed7a now show 62 successful and 9 skipped, with no failing or pending check runs. The CI rerun, including the macOS scan job, is green. My code approval still stands. Thanks for following up.

[sunchao]

Merged, thanks @peterxcli for the contribution!