Re: [DISCUSS] Changing namespace separator in REST spec

I do want to remind us that the original issue was reported by users from the community before we were internally aware of this issue, meaning that there are users of the V1 API that are either running into this issue or will eventually run into this

Re: [DISCUSS] Changing namespace separator in REST spec

I do object the plan to make that separation character configurable, because there is no need to do this (see below), so there's no need for any code change. Instead having a proper, mostly human readable escaping mechanism, which is possible, should be implemented with Iceberg REST v2. Here's

Re: [DISCUSS] Changing namespace separator in REST spec

I agree that the simplest approach would be to just pick a new namespace separator while still supporting the old one and be done with it, but this carries the risk that the namespace separator being chosen won't be the right one for everyone. Hence we're making the namespace separator configurabl

Re: [DISCUSS] Changing namespace separator in REST spec

It seems like our specification made a poor choice in terms of separator character, do we have any disagreement on this point? Not from me! But I think the next question is whether there is a better one for the general case so that we can just choose a new one and be done. I can’t think of a bette

Re: [DISCUSS] Changing namespace separator in REST spec

I feel like we are over complicating this situation. It seems like our specification made a poor choice in terms of separator character, do we have any disagreement on this point? It looks like by choosing a control character, we ended up generating requests which modern server systems define as p

Re: [DISCUSS] Changing namespace separator in REST spec

I do not see why "old clients are already broken" - they behave according to the REST spec. The current REST spec states "If parent is a multipart namespace, the parts must be separated by the unit separator (`0x1F`) byte" [1]. Using a different separator, even if it is "backwards compatible",

Re: [DISCUSS] Changing namespace separator in REST spec

@Dmitri thanks for the quick review. The query param is actually being handled in #10904 (spec) and in #10905 (impl) and is just a change on top of #10877 . E

Re: [DISCUSS] Changing namespace separator in REST spec

Thanks, Eduard, for 10877 [1] and addressing my concerns quickly :) This approach is fine from my POV, although I personally prefer the flexibility of what Jack proposed. Cheers, Dmitri. [1] https://github.com/apache/iceberg/pull/10877 On Thu, Aug 8, 2024 at 6:28 AM Eduard Tudenhöfner wrote:

Re: [DISCUSS] Changing namespace separator in REST spec

Thanks Eduard for bringing raising all the PRs. I like the approach of the server-side configuration, that way the catalog is in charge of providing a character that's suitable for them. Kind regards, Fokko Op do 8 aug 2024 om 12:27 schreef Eduard Tudenhöfner < etudenhoef...@apache.org>: > I've

Re: [DISCUSS] Changing namespace separator in REST spec

I've opened #10877 a few days ago to make the namespace separator configurable and let servers communicate to clients which separator should be used. Worth mentioning that this doesn't require any spec chance and it is backwards compatible with older c

Re: [DISCUSS] Changing namespace separator in REST spec

The idea of client-chosen separator char (?delim=.) sounds pretty reasonable to me. Nonetheless, I do not think this covers all the issues in putting namespaces in URI paths for servers running under the new Servlet spec. In particular, there are other chars that are considered "suspicious" by the

Re: [DISCUSS] Changing namespace separator in REST spec

Sorry a bit late to this thread. I would personally prefer the client side separator solution (query param with `?delim=.`) a bit more than the server side (config override), just given the experience of handling similar situations for Glue data catalog which allows any name for database (namespac

Re: [DISCUSS] Changing namespace separator in REST spec

1. Change %1F to %2E: As I noted in my earlier email, a catalog may choose to use . as a one-way conversion so that it doesn’t matter that you can’t split the namespace. This does work, but with slightly different behavior. The original decision on this issue was that the behavior should be a

Re: [DISCUSS] Changing namespace separator in REST spec

Thanks Ryan and Daniel for the context. I'm OK that the server provides a separator via config endpoint. Just want to dig a bit more, it does provide more flexibility for the separator, but why do we need this flexibility? Looks like the only benefit is to reconstruct the namespaces. What's the use

Re: [DISCUSS] Changing namespace separator in REST spec

It's probably best to make this configurable rather than changing the separator in the spec. The server can communicate to the client which namespace separator should be used (via a config override), but still needs to support *%1F* as a fallback. I've opened #10877

Re: [DISCUSS] Changing namespace separator in REST spec

I'd like to summarize the proposals that came up: 1. Change `%1F` to `%2E`: Already existing namespaces that have the dot-character (`%2E` == `.`) become inaccessible. A namespace ["my", "elem.foo"] is then encoded as `my.elem.foo` and decoded as ["my", "elem", "foo"], which is incorrect. 2.

Re: [DISCUSS] Changing namespace separator in REST spec

I would agree with adding either a server side (config override) or client side control (query param with `?delim=.`) as it will be compatible with the current v1 endpoint. In the future we could introduce a v2 endpoint(s), but I would want to wait for OpenAPI 4 because they address this by allowi

Re: [DISCUSS] Changing namespace separator in REST spec

Just curious—why did we originally introduce %1F as a separator? When we were first discussing how to send multi-part namespace identifiers, there was a choice about how to encode them. I advocated for using . as we do for column names, but there were people that felt strongly about needing to use

Re: [DISCUSS] Changing namespace separator in REST spec

Just curious—why did we originally introduce %1F as a separator? Was it because we wanted to allow "." as a valid character in namespaces? If that’s the case, I get that we couldn't use "." or "%2e" as a separator. A follow up question: can we restrict the character "." in a namespace name? For e

Re: [DISCUSS] Changing namespace separator in REST spec

I'd be very careful here. The strings in `Namespace` elements are unconstrained. Neither the `Namespace` implementation in Iceberg/Java nor the REST spec restrict the contents of the namespace elements. So a '.' can appear in existing namespace elements and choosing %2E breaks such existing na

Re: [DISCUSS] Changing namespace separator in REST spec

+1 on the first option. We may not overly use the config endpoint, but it'd be suitable in this case. We can introduce a new field like this: namespace.separator=%2e Yufei On Thu, Aug 1, 2024 at 3:46 PM Ryan Blue wrote: > I think the simplest way to preserve compatibility is to allow this to

Re: [DISCUSS] Changing namespace separator in REST spec

I think the simplest way to preserve compatibility is to allow this to be configured on the client and by the config route, and fall back to the current value, 0x1f. Another option is to introduce a set of v2 endpoints that use a different separator character. I prefer the first option since the on

Re: [DISCUSS] Changing namespace separator in REST spec

How is compatibility with older servers guaranteed? On 01.08.24 14:59, Eduard Tudenhöfner wrote: Hey everyone, The REST spec currently uses *%1F* as the UTF-8 encoded nam

Re: [DISCUSS] Changing namespace separator in REST spec

+1 for replacing it to be compatible with the new Servlet spec. Yufei On Thu, Aug 1, 2024 at 7:02 AM Eduard Tudenhöfner wrote: > Here's the PR that bumps > Jetty and the Servlet API and reproduces #10338 >

Re: [DISCUSS] Changing namespace separator in REST spec

Here's the PR that bumps Jetty and the Servlet API and reproduces #10338 but it unfortunately requires to be built/executed with JDK17. On Thu, Aug 1, 2024 at 2:59 PM Eduard Tudenhöfner wrote: > Hey e

[DISCUSS] Changing namespace separator in REST spec

Hey everyone, The REST spec currently uses *%1F* as the UTF-8 encoded namespace separator for multi-part namespaces. This causes issues