Skip to content

Spark SQL Writer Tests (manual) never runs Comet's native writer #6749

Description

@andygrove

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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:ciCI/CD, GitHub Actions, build toolingarea:writerNative Parquet writerbugSomething isn't workingrequires-triage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions