You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by "dkling-it (via GitHub)" <gi...@apache.org> on 2026/08/21 20:48:22 UTC

[PR] [SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

dkling-it opened a new pull request, #58219:
URL: https://github.com/apache/spark/pull/58219

### What changes were proposed in this pull request?
Renames `_LEGACY_ERROR_TEMP_2033` to `UNABLE_TO_CREATE_DATABASE`. It's raised by `QueryExecutionErrors.unableToCreateDatabaseAsFailedToCreateDirectoryError` when `InMemoryCatalog.createDatabase` can't create the database's backing directory. SQLSTATE `58030` follows the same convention as other filesystem I/O failures such as `FAILED_CREATE_CHECKPOINT_DIRECTORY` and `UNABLE_TO_FETCH_HIVE_TABLES`. Message text and parameters are unchanged.

### Why are the changes needed?
Part of the SPARK-37935 effort to replace `_LEGACY_ERROR_TEMP_*` placeholders with proper error conditions.

### Does this PR introduce _any_ user-facing change?
Yes. The error condition name changes from `_LEGACY_ERROR_TEMP_2033` to `UNABLE_TO_CREATE_DATABASE` and now carries SQLSTATE `58030`. The rendered message is unchanged.

### How was this patch tested?
Added a `checkError` test in `InMemoryCatalogSuite` that points `createDatabase` at a location nested under an existing regular file, so directory creation fails and the new condition fires. Ran `InMemoryCatalogSuite` and `SparkThrowableSuite` locally; both pass.

### Was this patch authored or co-authored using generative AI tooling?
No.

--
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.

nchammas commented on code in PR #58219:
URL: https://github.com/apache/spark/pull/58219#discussion_r3895186444


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalogSuite.scala:
##########
@@ -28,4 +30,24 @@ class InMemoryCatalogSuite extends ExternalCatalogSuite {
     override def newEmptyCatalog(): ExternalCatalog = new InMemoryCatalog
   }
 
+  test("createDatabase throws UNABLE_TO_CREATE_DATABASE_DIRECTORY when mkdirs fails") {
+    val parentFile = Utils.createTempDir()

Review Comment:
   Won't this leave the temp dir lying around? I think you want to [use `withTempDir` instead](https://github.com/apache/spark/blob/9911af1935efdaac1fcbcde760719f75228f7416/core/src/test/scala/org/apache/spark/SparkTestSuite.scala#L191).



-- 
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] [SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.

nchammas commented on code in PR #58219:
URL: https://github.com/apache/spark/pull/58219#discussion_r3838860918


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -582,7 +582,7 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
   def unableToCreateDatabaseAsFailedToCreateDirectoryError(
       dbDefinition: CatalogDatabase, e: IOException): Throwable = {
     new SparkException(
-      errorClass = "_LEGACY_ERROR_TEMP_2033",
+      errorClass = "UNABLE_TO_CREATE_DATABASE",

Review Comment:
   This error condition is more general than the actual failure path here. Why not `UNABLE_TO_CREATE_DATABASE_DIRECTORY`?
   
   (Unfortunately, the code still uses "error class" in many places to refer to what is correctly called an "error condition".)



-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "dkling-it (via GitHub)" <gi...@apache.org>.

dkling-it commented on PR #58219:
URL: https://github.com/apache/spark/pull/58219#issuecomment-5572582579

   @uros-b @nchammas this has been approved since Aug 31 and the build is green. Anything else needed from my side before it can be merged?


-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.

uros-b commented on PR #58219:
URL: https://github.com/apache/spark/pull/58219#issuecomment-5582500921

   Thank you @dkling-it and @nchammas! The rename is clean, the condition name is meaningful, the SQLSTATE 58030 matches the filesystem-I/O peer convention, the message text is unchanged, the alphabetical insertion point in error-conditions.json is correct, and the test exercises the IOException path correctly.


-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "dkling-it (via GitHub)" <gi...@apache.org>.

dkling-it commented on PR #58219:
URL: https://github.com/apache/spark/pull/58219#issuecomment-5479126264

   Filed [SPARK-59129](https://issues.apache.org/jira/browse/SPARK-59129) as a sub-task of SPARK-37935 and put it in the title. Sorry for the delay, the account request took a while to come through.
   
   @nchammas you're right about the name. Both the message and the only call site are about the directory, not the database, so it's now `UNABLE_TO_CREATE_DATABASE_DIRECTORY` (dcf0f930). It also pairs with `_LEGACY_ERROR_TEMP_2034` next door, which is the drop-the-directory counterpart and will presumably become `UNABLE_TO_DROP_DATABASE_DIRECTORY`.
   
   `SparkThrowableSuite` and `InMemoryCatalogSuite` pass locally.
   


-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.

uros-b commented on PR #58219:
URL: https://github.com/apache/spark/pull/58219#issuecomment-5582625121

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/9a18c2fabc6ede61e56176ca7b56b4e2a20391dc
   - merged into branch-4.x https://github.com/apache/spark/commit/c0f6b9820c01bf440eaa32792048b66b2c93671a
   
   *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

Re: [PR] [SPARK-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.

uros-b closed pull request #58219: [SPARK-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033
URL: https://github.com/apache/spark/pull/58219


-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "dkling-it (via GitHub)" <gi...@apache.org>.

dkling-it commented on code in PR #58219:
URL: https://github.com/apache/spark/pull/58219#discussion_r3894955137


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -582,7 +582,7 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
   def unableToCreateDatabaseAsFailedToCreateDirectoryError(
       dbDefinition: CatalogDatabase, e: IOException): Throwable = {
     new SparkException(
-      errorClass = "_LEGACY_ERROR_TEMP_2033",
+      errorClass = "UNABLE_TO_CREATE_DATABASE",

Review Comment:
   Renamed to `UNABLE_TO_CREATE_DATABASE_DIRECTORY` in dcf0f930. Agreed on the reasoning, the failing operation is the directory rather than the database.



-- 
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-59129][SQL] Assign a name to the error condition _LEGACY_ERROR_TEMP_2033 [spark]

Posted by "dkling-it (via GitHub)" <gi...@apache.org>.

dkling-it commented on code in PR #58219:
URL: https://github.com/apache/spark/pull/58219#discussion_r3896396193


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalogSuite.scala:
##########
@@ -28,4 +30,24 @@ class InMemoryCatalogSuite extends ExternalCatalogSuite {
     override def newEmptyCatalog(): ExternalCatalog = new InMemoryCatalog
   }
 
+  test("createDatabase throws UNABLE_TO_CREATE_DATABASE_DIRECTORY when mkdirs fails") {
+    val parentFile = Utils.createTempDir()

Review Comment:
   Right, changed in afbd33bb. `withTempDir` deletes the directory when the test returns instead of leaving it to the shutdown hook. `InMemoryCatalogSuite` still passes, 73 tests.



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