andygrove opened a new issue, #6749:
URL: https://github.com/apache/datafusion-comet/issues/6749
### 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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
