ykisana opened a new pull request, #59333:
URL: https://github.com/apache/spark/pull/59333

   ### What changes were proposed in this pull request?
   
   In `FileFormatWriter.write`, set `spark.sql.sources.writeJobUUID` on the job 
configuration before `committer.setupJob(job)` instead of after 
`materializeAdaptiveSparkPlan`, inside the `try` block.
   
   SPARK-56919 moved `committer.setupJob(job)` ahead of 
`materializeAdaptiveSparkPlan(plan)` so that an AQE failure calls 
`committer.abortJob(job)` instead of losing the output path. The `writeJobUUID` 
assignment stayed where it was, so committers now run `setupJob` (and 
`abortJob`, if materialization fails) before the UUID is set. This PR restores 
the pre-SPARK-56919 order: set the UUID, then call `setupJob`. 
`SparkHadoopWriter` uses the same order for the RDD write path. 
`description.uuid` is already available at that point, and the value is 
unchanged.
   
   ### Why are the changes needed?
   
   Committers that read `spark.sql.sources.writeJobUUID` in `setupJob` or 
`abortJob` fall back to `jobContext.getJobID()` when it is not set. That job ID 
is null for the `Job` that `FileFormatWriter` creates, because the job is never 
submitted. Hadoop's `ManifestCommitter`, the default committer for `gs://` and 
`abfs://` since Hadoop 3.4, fails every file source write with:
   
   ```
   java.lang.NullPointerException: Cannot invoke 
"org.apache.hadoop.mapreduce.JobID.toString()" because "jobId" is null
     at 
org.apache.hadoop.mapreduce.lib.output.committer.manifest.impl.ManifestCommitterSupport.buildJobUUID(ManifestCommitterSupport.java:118)
     at 
org.apache.hadoop.mapreduce.lib.output.committer.manifest.ManifestCommitterConfig.<init>(ManifestCommitterConfig.java:190)
     at 
org.apache.hadoop.mapreduce.lib.output.committer.manifest.ManifestCommitter.enterCommitter(ManifestCommitter.java:181)
     at 
org.apache.hadoop.mapreduce.lib.output.committer.manifest.ManifestCommitter.setupJob(ManifestCommitter.java:196)
     at 
org.apache.spark.internal.io.HadoopMapReduceCommitProtocol.setupJob(HadoopMapReduceCommitProtocol.scala:198)
     at 
org.apache.spark.sql.execution.datasources.FileFormatWriter$.write(FileFormatWriter.scala:165)
   ```
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, a bug fix. SPARK-56919 shipped in Spark 4.2.0, and since then file 
source writes (`DataFrameWriter`, `INSERT`, Hive `INSERT` and the file 
streaming sink) fail with the `NullPointerException` above when the output 
committer reads `spark.sql.sources.writeJobUUID` during job setup, e.g. 
`ManifestCommitter`. After this change, these writes succeed as they did before 
4.2.0.
   
   ### How was this patch tested?
   
   Added a test, `SPARK-60023: writeJobUUID should be set before setupJob`, to 
`FileFormatWriterSuite`. It sets `spark.sql.sources.commitProtocolClass` to a 
test `SQLHadoopMapReduceCommitProtocol` subclass whose `setupJob` asserts that 
`spark.sql.sources.writeJobUUID` is set, then writes and reads back a parquet 
file.
   
   ```
   build/sbt 'sql/testOnly *FileFormatWriterSuite -- -z "SPARK-60023"'
   ```
   
   - With this change, both the codegen and interpreted variants pass.
   - Without the change in `FileFormatWriter.scala`, both fail with `assertion 
failed: spark.sql.sources.writeJobUUID should be set before setupJob`.
   
   `build/sbt sql/scalastyle sql/Test/scalastyle` also passes.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated by: Claude Opus 5.5
   Fix written by and all code manually reviewed by @ykisana.
   Tests, PR description, and comments by Claude.
   


-- 
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]

Reply via email to