Adjust the "table_exists" behavior in the REST Catalog

[ndrluis]

Apache Iceberg version

0.7.0 (latest release)

Please describe the bug 🐞

Currently, we return True when the status code is 200 or 204, and False for all other status codes. According to the REST specification, we should return False when the catalog returns a 404 status code and raise an error for other status codes.

In the Java implementation, a try/catch block is used with load_table, and when it catches a NoSuchTableError, it currently returns False.

I believe that this behavior is suppressing errors and looks like it's a bug as reported in #1006

@kevinjqliu I noticed that you are looking into the Issue on the Polaris side and I believe that we need to adjust this on our side.

[kevinjqliu]

Thanks for looking into this. The behavior you described above seems like the correct one to me.

I believe that this behavior is suppressing errors and looks like it's a bug as reported in #1006

I don't follow this since the expected value should be True. According to the spec, the function returns True only for the 204 status code.
There's a theory in the other thread (apache/polaris#96 (comment)) that the error is due to managed Polaris not implementing the HEAD API, which would cause any HEAD request to return 404.

Reference

• tableExists REST spec
• table_exists PyIceberg REST client
• tableExists Java client

[TiansuYu]

@ndrluis @kevinjqliu
I can contribute to this. Would you say the intended behavior should be:

• 204 return True
• 404 NoSuchTableException
• others: raise a corresponding response error

[ndrluis]

Does it make sense for a method suffixed with "exists" to never return false? As I mentioned, and as it is in the Java implementation, when a 404 status is returned, it should indicate false.

However, as @kevinjqliu pointed out, we might receive a 404 when the HEAD method is not implemented. I believe we should expect the Specification to be implemented as defined, and if it's not, it's a bug on the catalog side and not on ours.

Therefore, I believe we can follow the Java implementation in the sense that it returns false when a 404 status is encountered, but we should maintain the use of the HEAD call because it is expected to exist according to the Catalog spec.

@kevinjqliu @TiansuYu @Fokko @sungwy @HonahX What are your thoughts?

[sungwy]

Hi @TiansuYu and @ndrluis - thank you for bringing up this point, and sorry for not getting around to looking at this earlier.

Similar to what @TiansuYu suggested, I'm of the opinion that we should do the following:

• 200, 204: return True
• 404: return False
• others, raise a corresponding response error.

The reason is, because there's a numerous different factors as to whether a REST endpoint will return a non non 204/404 response, and it would be erroneous for us to return false on any other status code. If users are relying on this endpoint to return affirmatively say that the table exists or not for their use case, and if the REST catalog returns a 5xx error due to an unknown reason, having that be interpreted as False is error-prone

[sungwy]

@ndrluis @kevinjqliu I can contribute to this. Would you say the intended behavior should be:

• 204 return True
• 404 NoSuchTableException
• others: raise a corresponding response error

It looks like @ndrluis raised this issue, and has it assigned this to himself. Maybe we could help review his code :)

[Fokko]

Thanks everyone for bringing this up.

Long term, I think we should leverage the endpoint discovery to see if the server provides the capability of table-exists: https://lists.apache.org/thread/8h86382omdx9cmvc15m2bf361p5rz4rk

Since the table-exists is in there from the beginning, I think we should follow the spec, and assume that:

• 204: Table exists.
• 404: Table doesn't exist.
• Raise an exception for the rest, similar to other operations.

The HEAD operation does not require the REST Catalog to fetch the metadata, which will consume less resources on the server, but probably also return much faster for the client.

I think need to update the Java implementation to use the HEAD request, this will also uncover the issue on the Java side. For validation, hopefully, we can check this properly using apache/iceberg#10908 once that is in.

[Fokko]

I checked the Java side, and I could not find the HEAD request. For raising visibility, I created an issue there: apache/iceberg#10993

[kevinjqliu]

@ndrluis can this be closed now that #1096 is merged?