You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dongjoon-hyun (via GitHub)" <gi...@apache.org> on 2026/10/20 15:00:26 UTC

[PR] [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing [spark]

dongjoon-hyun opened a new pull request, #59643:
URL: https://github.com/apache/spark/pull/59643

   [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing
   
   ### What changes were proposed in this pull request?
   
   This PR makes `saveAsTable` with `SaveMode.Overwrite` drop the existing V1 table only after the checks that run before writing, such as the analysis, the data schema check, and the write options.
   
   Previously, `SaveAsV1TableCommand` dropped the table before running the CTAS. Now the CTAS drops it:
   - `CreateDataSourceTableAsSelectCommand` drops it right before the write job starts, in the `beforeSetupJob` callback of `FileFormatWriter.write` (SPARK-60360), or right before `CreatableRelationProvider.createRelation`.
   - `CreateHiveTableAsSelectCommand` drops it after the analysis, because it creates the new table before writing.
   
   ### Why are the changes needed?
   
   When these checks fail, the table has already been dropped. A managed table loses its data too.
   
   ```scala
   spark.range(3).write.saveAsTable("t")
   spark.range(5).write.mode("overwrite").option("compression", "bogus").saveAsTable("t")
   // SparkIllegalArgumentException: [CODEC_NOT_AVAILABLE.WITH_AVAILABLE_CODECS_SUGGESTION] ...
   spark.table("t")
   // AnalysisException: [TABLE_OR_VIEW_NOT_FOUND] The table or view `t` cannot be found. ...
   ```
   
   The same happens with an unsupported data type (e.g. a map column in CSV) or a nonexistent partition column. This has been the behavior since Spark 1.3.0.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. Such an overwrite keeps the existing table and its data. Valid writes are unchanged.
   
   ### How was this patch tested?
   
   Added tests to `DataFrameReaderWriterSuite` and `MetastoreDataSourcesSuite`. They fail without this fix.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Opus 5.5


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59643:
URL: https://github.com/apache/spark/pull/59643#issuecomment-6297525123

   Thank you for the review and approval, @peter-toth .
   
   1. You're right. I updated the "user-facing change" section of the PR description for `format("hive")`.
   
   2. Yes, this is the same as before this PR. I'd like to keep this PR as it is because these cases are rare, and the exact condition needs to compare the location of the existing table with the new one. A wrong mismatch there would make a normal overwrite of a managed table fail with `LOCATION_ALREADY_EXISTS`.
   
   3. Good point. This can happen today, not only after a future rewrite. For example, the experimental native Parquet writer of Apache DataFusion Comet replaces `DataWritingCommandExec(InsertIntoHadoopFsRelationCommand)` with its own operator, so the callback never runs. A loud failure after the write would still leave the old table entry pointing at the new files. So, `DataSource.writeAndRead` now drops the table before the write, as before this PR, when the executed plan has no `InsertIntoHadoopFsRelationCommand` with the tag. I added a test with a planner strategy that drops the tag.
   
   4. Fixed.
   
   5. Fixed in both `MetastoreDataSourcesSuite` and `DataFrameReaderWriterSuite`.
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59643:
URL: https://github.com/apache/spark/pull/59643#issuecomment-6293952007

   Hi, @peter-toth . This is the one you requested in the previous comment, https://github.com/apache/spark/pull/59610#pullrequestreview-5581702211 .


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #59643: [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing
URL: https://github.com/apache/spark/pull/59643


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60387][SQL] Keep the existing table when `saveAsTable` overwrite fails before writing [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59643:
URL: https://github.com/apache/spark/pull/59643#issuecomment-6300854135

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/5637184e54a25ab628f27fcea0f40ea253d63e56
   - merged into branch-4.x https://github.com/apache/spark/commit/6777f8517d07f5da45dc548898567f5e1e258ec2
   - merged into branch-4.3 https://github.com/apache/spark/commit/8a7c856dcdf1fbe9c502a8c9c34c66d88ba0032c
   
   *Posted by `merge_spark_pr.py`*


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org