apache / iceberg

Apache Iceberg

https://iceberg.apache.org/

Apache License 2.0

6.49k stars 2.24k forks source link

Revert "Core: Use encoding/decoding methods for namespaces and deprecate Splitter/Joiner" #11574

Closed nastra closed 2 days ago

nastra commented 3 days ago

Reverts apache/iceberg#10858

I'll revert this one for now and do a proper fix for 1.8.0

bryanck commented 3 days ago

I'm not convinced we need to revert this. In my test env I have Trino 465 + 1.7 with nested namespaces enabled, connecting to an Iceberg 1.6 REST catalog. I'm seeing the same error in the REST catalog: Unhandled error: ErrorResponse(code=404, type=NoSuchNamespaceException, message=Namespace does not exist: bck%1Fdb1) whether Trino is using 1.7.0 or this branch. Perhaps we can assume that if you have nested namespaces enabled in Trino you need a REST catalog with 1.7?

nastra commented 2 days ago

@bryanck the issue is that the 1.7 client sends the query param (where the nested namespace is encoded) using %1F but the 1.6 catalog expects the non-UTF-8 encoded character \u001f. Basically Iceberg 1.6 prior to #10858 used RestUtil.encodeNamespace() / RestUtil.decodeNamespace() only for paths and used \u001f directly with a Splitter/Joiner for the query param on the client and the server, which effectively causes the issue.

10877, so I'll revisit the approach but in the meantime it's probably the safest to just revert.

bryanck commented 2 days ago

I retested this and this does resolve the issue (I tested the wrong Trino build previously).

nastra commented 2 days ago

thanks for reviewing and testing this @bryanck

mayankvadariya commented 2 days ago