[andygrove]

Which issue does this PR close?

Closes #5112.
Closes #7494.

Rationale for this change

The native columnar-to-row converter (CometNativeColumnarToRowExec) has been off by default since #5114. In the discussion on #4440, nobody could measure a benefit from it. An SF1000 cluster comparison showed the same throughput, GC time and peak memory as the JVM CometColumnarToRowExec. #5112 shows that in isolation it is 3.7x slower per row at 8192-row batches, and the gap grows as batches get smaller. The reason is the fixed cost of the FFI export, the JNI call and the import that every batch pays. The JVM operator also supports whole-stage codegen, so it fuses into its consumer's stage. The native operator can't do that, so it always ends the stage.

Keeping the converter still costs something. It is about 2,700 lines of Rust plus a JVM operator, and every transition rule, test and Spark SQL diff has to handle it. It also has a critical open bug: row offsets wrap past 2 GiB (#7494). Deleting it fixes that bug and makes #7504 unnecessary.

What changes are included in this PR?

• Remove native/core/src/execution/columnar_to_row.rs, the columnarToRowInit / columnarToRowConvert / columnarToRowClose JNI entry points and their tests, NativeColumnarToRowInfo, NativeColumnarToRowConverter, CometNativeColumnarToRowExec and CometNativeColumnarToRowSuite.
• EliminateRedundantTransitions now always creates CometColumnarToRowExec. A schema that contains Variant still keeps Spark's ColumnarToRowExec, which is now tagged with the fallback reason directly rather than rebuilt. The rules and tests that matched both Comet transitions now match only the JVM one.
• spark.comet.exec.columnarToRow.native.enabled is in the exec category, so the versioning policy requires a deprecation cycle before it can be removed. It stays registered as a deprecated no-op, and the driver logs a warning when it is set in the Spark configuration. The driver's warning for the deprecated spark.comet.exec.memoryPool.fraction now goes through the same code: CometDriverPlugin.warnIfDeprecatedConfigsSet, which reads a list of deprecated keys with a reason for each. The upgrade guide has a new 1.3.0 section for this.
• CometFuzzTestBase used to run each test with native shuffle, and with JVM shuffle under both C2R settings. It now runs once per shuffle mode.
• The benchmarks drop their native-converter cases. CometStringFfiImportBenchmark drops its columnar-to-row case, because that import site no longer exists.
• The five Spark SQL test diffs are regenerated from patched Spark checkouts. They drop the CometNativeColumnarToRowExec match arms and imports in SubquerySuite, SparkPlanSuite and BucketedReadSuite. For 4.0 and later they also drop the VariantEndToEndSuite hunk, which existed only for this operator. Before any edit, each diff round-tripped byte for byte, and each new diff applies cleanly to its tag.
• Docs, comments, the CI suite lists and the labeler are updated.

How are these changes tested?

The existing tests cover this. CometPluginsDeprecatedConfigWarningSuite replaces CometPluginsMemoryPoolFractionWarningSuite in the CI suite lists, and it checks the warning for both deprecated keys. I ran these locally on the default Spark 4.1 profile: CometPluginsDeprecatedConfigWarningSuite, EliminateRedundantTransitionsSuite, RevertNativeForTransitionHeavyStagesSuite, CometDirectColumnarToRowSuite, CometVariantTypeSuite, CometVariantProjectionSuite, CometInMemoryCacheSuite, CometFuzzTestSuite, CometExecRuleSuite, and the touched tests in CometExecSuite and CometParquetWriterSuite. All 308 tests passed. cargo clippy --all-targets --workspace -- -D warnings is clean.

The Spark SQL diffs changed, so the Spark SQL suites need a run before this leaves draft.

[mbutrovich]

Thanks @andygrove for investigating in #7506. I checked the leftover references, the deprecation handling, and the regenerated Spark SQL diffs, and found nothing that needs a change.

[comphead]

Thanks for the thorough removal. I grepped the PR head for NativeColumnarToRow, the JNI names, COMET_NATIVE_COLUMNAR_TO_ROW_ENABLED and the config key and found nothing left except the deprecated entry, its warning, the upgrade guide and a historical result file under benchmarks/results/1.0.0.

One leftover that has no anchor in this diff. NativeUtil.exportBatchToAddresses and NativeUtil.exportBatch in spark/src/main/scala/org/apache/comet/vector/NativeUtil.scala look unused by production code now. On main the only caller of exportBatchToAddresses was NativeColumnarToRowConverter, and exportBatch is only called from exportBatchToAddresses. At this head the remaining callers are NativeUtilSuite and CometUdfBridgeSuite. Could they be removed here, along with their ConstantColumnVector materialization arm and the exportBatch test in NativeUtilSuite? CometUdfBridgeSuite would need another way to build its input addresses, and allocateArrowStructs is still used by getNextBatch. The stale mentions of NativeUtil.exportBatch in CometMapInBatchExec.scala (line 109) would then need updating too. If you prefer to keep this PR to the converter, a follow-up issue would be enough.

Two open PRs build on what this removes. #7124 adds a test to the deleted CometNativeColumnarToRowSuite and fixes exportBatch for this path, and the draft #6589 adds Variant support to the removed converter, so #5436 would now need the JVM converter.

[comphead]

This branch now tags the original ColumnarToRowExec instead of building a new one, and the PR also removes the two Variant assertions that referenced the native converter. I could not find a test that checks the Variant ColumnarToRowExec carries this reason. CometVariantProjectionSuite.checkNative only excludes ColumnarToRowExec from the Comet-operator check. Would it be worth asserting in checkNative that new ExtendedExplainInfo().getFallbackReasons(plan) contains Comet columnar-to-row conversion does not support type VariantType? I have not run it, so I do not know whether another node already contributes a matching reason.