Describe the bug
The manual Spark SQL Writer Tests (manual) workflow (.github/workflows/spark_sql_writer_tests.yml) is meant to run Spark's Parquet writer suites with Comet's native writer on. It never does. The workflow sets ENABLE_COMET_WRITE=true, which only changes the default of spark.comet.write.parquet.enabled. The native write reports Incompatible (on Spark 3.x through CometDataWritingCommand, on 4.0+ through CometWriteFiles), so the planner also needs spark.comet.operator.DataWritingCommandExec.allowIncompatible=true (3.x) or spark.comet.operator.WriteFilesExec.allowIncompatible=true (4.0+). Neither the workflow nor the patched SharedSparkSession in dev/diffs/*.diff sets them, so every write in a green run goes through Spark's own writer.
Setting the documented env var does not help either. createOperatorIncompatConfig gives each operator key an env-var default (SPARK_COMET_OPERATOR_DATAWRITINGCOMMANDEXEC_ALLOWINCOMPATIBLE, and the entry's doc says it "can be overridden" that way), but CometConf.isOperatorAllowIncompat reads only the SQL conf string through readWithAlternatives and never consults the entry's default. With both env vars set, COMET_OPERATOR_DATA_WRITING_COMMAND_ALLOW_INCOMPAT.get() returns true while isOperatorAllowIncompat("DataWritingCommandExec") returns false, and the write stays on InsertIntoHadoopFsRelationCommand.
Found while reviewing #4746. Its 3.5 run (peterxcli/datafusion-comet run 37306706506) passed 377 tests, and the log never shows the Comet supports DataWritingCommandExec when ... but has notes warning that CometExecRule prints when it admits a native write.
Steps to reproduce
Run the workflow, or locally run any Spark writer suite with ENABLE_COMET=true ENABLE_COMET_WRITE=true and check the executed plans. They contain Execute InsertIntoHadoopFsRelationCommand, not CometNativeWrite (3.x) or CometWriteFiles (4.0+).
Expected behavior
The workflow runs the native writer. Two ways to get there: make isOperatorAllowIncompat honor the entry's env-var default and set the operator env var in the workflow, or pass the operator key to the forked test JVM. SparkBuild forwards only http.*/https.* system properties, so plain -D flags on build/sbt do not reach the forked JVM.
Even with the writer on, coverage is thin. Most of these suites write from local data, which Comet does not run natively, so the write falls back on requiresNativeChildren. Running the 3.5.9 suites from the test jar with the native writer on, only 10 tests committed a native write.
Additional context
Local runs of Spark's test jar (unpatched) with the writer on show one writer-specific failure, "Write Spark version into Parquet metadata" (#3427).
Describe the bug
The manual
Spark SQL Writer Tests (manual)workflow (.github/workflows/spark_sql_writer_tests.yml) is meant to run Spark's Parquet writer suites with Comet's native writer on. It never does. The workflow setsENABLE_COMET_WRITE=true, which only changes the default ofspark.comet.write.parquet.enabled. The native write reportsIncompatible(on Spark 3.x throughCometDataWritingCommand, on 4.0+ throughCometWriteFiles), so the planner also needsspark.comet.operator.DataWritingCommandExec.allowIncompatible=true(3.x) orspark.comet.operator.WriteFilesExec.allowIncompatible=true(4.0+). Neither the workflow nor the patchedSharedSparkSessionindev/diffs/*.diffsets them, so every write in a green run goes through Spark's own writer.Setting the documented env var does not help either.
createOperatorIncompatConfiggives each operator key an env-var default (SPARK_COMET_OPERATOR_DATAWRITINGCOMMANDEXEC_ALLOWINCOMPATIBLE, and the entry's doc says it "can be overridden" that way), butCometConf.isOperatorAllowIncompatreads only the SQL conf string throughreadWithAlternativesand never consults the entry's default. With both env vars set,COMET_OPERATOR_DATA_WRITING_COMMAND_ALLOW_INCOMPAT.get()returnstruewhileisOperatorAllowIncompat("DataWritingCommandExec")returnsfalse, and the write stays onInsertIntoHadoopFsRelationCommand.Found while reviewing #4746. Its 3.5 run (
peterxcli/datafusion-cometrun 37306706506) passed 377 tests, and the log never shows theComet supports DataWritingCommandExec when ... but has noteswarning thatCometExecRuleprints when it admits a native write.Steps to reproduce
Run the workflow, or locally run any Spark writer suite with
ENABLE_COMET=true ENABLE_COMET_WRITE=trueand check the executed plans. They containExecute InsertIntoHadoopFsRelationCommand, notCometNativeWrite(3.x) orCometWriteFiles(4.0+).Expected behavior
The workflow runs the native writer. Two ways to get there: make
isOperatorAllowIncompathonor the entry's env-var default and set the operator env var in the workflow, or pass the operator key to the forked test JVM.SparkBuildforwards onlyhttp.*/https.*system properties, so plain-Dflags onbuild/sbtdo not reach the forked JVM.Even with the writer on, coverage is thin. Most of these suites write from local data, which Comet does not run natively, so the write falls back on
requiresNativeChildren. Running the 3.5.9 suites from the test jar with the native writer on, only 10 tests committed a native write.Additional context
Local runs of Spark's test jar (unpatched) with the writer on show one writer-specific failure, "Write Spark version into Parquet metadata" (#3427).