[Spark Integration Tests] TestCreateTable::testCreateTableCommitProperties won't work on RESTCatalog

[haizhou-zhao]

Apache Iceberg version

None

Query engine

None

Please describe the bug 🐞

Part of: #11079

Intro

The test TestCreateTable::testCreateTableCommitProperties will only work on Hadoop/Hive Catalog but not on REST Catalog. Code link: https://github.com/apache/iceberg/blame/0a705b0637db484730eb4eece69ae6c4d52fd9da/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java#L353

Error message

TestCreateTable > testCreateTableCommitProperties() > catalogName = testrest, implementation = org.apache.iceberg.spark.SparkCatalog, config = {type=rest, cache-enabled=false, uri=http://localhost:34885/} FAILED
    java.lang.AssertionError: 
    Expecting actual throwable to be an instance of:
      org.apache.iceberg.exceptions.ValidationException
    but was:
      org.apache.iceberg.exceptions.BadRequestException: Malformed request: Table property commit.retry.num-retries must have integer value
    	at org.apache.iceberg.rest.ErrorHandlers$DefaultErrorHandler.accept(ErrorHandlers.java:208)
    	at org.apache.iceberg.rest.ErrorHandlers$TableErrorHandler.accept(ErrorHandlers.java:118)
    	at org.apache.iceberg.rest.ErrorHandlers$TableErrorHandler.accept(ErrorHandlers.java:102)
    	...(164 remaining lines not displayed - this can be changed with Assertions.setMaxStackTraceElementsDisplayed)
        at org.apache.iceberg.spark.sql.TestCreateTable.testCreateTableCommitProperties(TestCreateTable.java:366)

Root cause

REST client will throw BadRequestException (HTTP 400) where a ValidationException will be thrown by Hive/Hadoop Catalog in similar situation. REST client will simply categorize ValidationException as HTTP 400 and throw BadRequestException instead. This is currently determined by REST's ErrorHandlers:

iceberg/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java

Line 202 in 0a705b0

public void accept(ErrorResponse error) {

Further consideration

Two options going forward:

1. Unify Hive/Hadoop/REST interface so all of them throw the same type of exception in the cases that this test is covering for
2. Accept that Hive/Hadoop/REST could throw different type of exception in these cases and modify the test itself

Willingness to contribute

• I can contribute a fix for this bug independently
• I would be willing to contribute a fix for this bug with guidance from the Iceberg community
• I cannot contribute a fix for this bug at this time

[haizhou-zhao]

cc: @dramaticlly (author of the test in the issue)

[dramaticlly]

thank you @haizhou-zhao , I realized we actually have similar special handling for IllegalArgumentException per #9225 and I am wondering if it's worth doing the same for ValidationException.

The original reason I use VE instead of IAE is because it's already being used in
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L135
and
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/MetricsConfig.java#L204-L213

I think 400 generally is correct behaviour as user need to change their input, as of now the easier thing is probably add assumeThat so that this particular test will only run against the hive/hadoop catalog