This is an automated email from the ASF dual-hosted git repository.
dweeks pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 8bb66f7071 Core: Deprecate Namespace Joiner/Splitter and use separate
methods (#14274)
8bb66f7071 is described below
commit 8bb66f7071cacfb69f37d72807836b5a841df0ea
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Wed Oct 8 17:20:35 2025 +0200
Core: Deprecate Namespace Joiner/Splitter and use separate methods (#14274)
This revisits https://github.com/apache/iceberg/pull/10858 (which was
reverted in https://github.com/apache/iceberg/pull/11574) and uses separate
method to join/split a namespace using the unicode character \001f.
We eventually want to make the namespace separator configurable and in
order to do that, all namespace joining/splitting needs to go through
respective methods instead of using the Joiner/Splitter directly
---
.../apache/iceberg/rest/RESTSessionCatalog.java | 2 +-
.../java/org/apache/iceberg/rest/RESTUtil.java | 49 +++++++++++++++++++++-
.../apache/iceberg/rest/RESTCatalogAdapter.java | 6 +--
.../java/org/apache/iceberg/rest/TestRESTUtil.java | 46 ++++++++++++++++++++
4 files changed, 95 insertions(+), 8 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
index 4692b92b51..b903f13adc 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
@@ -570,7 +570,7 @@ public class RESTSessionCatalog extends
BaseViewSessionCatalog
Map<String, String> queryParams = Maps.newHashMap();
if (!namespace.isEmpty()) {
- queryParams.put("parent",
RESTUtil.NAMESPACE_JOINER.join(namespace.levels()));
+ queryParams.put("parent", RESTUtil.namespaceToQueryParam(namespace));
}
ImmutableList.Builder<Namespace> namespaces = ImmutableList.builder();
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
index 01b57a4151..d31260a918 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
@@ -32,13 +32,23 @@ import
org.apache.iceberg.relocated.com.google.common.collect.Maps;
public class RESTUtil {
private static final char NAMESPACE_SEPARATOR = '\u001f';
- public static final Joiner NAMESPACE_JOINER = Joiner.on(NAMESPACE_SEPARATOR);
- public static final Splitter NAMESPACE_SPLITTER =
Splitter.on(NAMESPACE_SEPARATOR);
private static final String NAMESPACE_ESCAPED_SEPARATOR = "%1F";
private static final Joiner NAMESPACE_ESCAPED_JOINER =
Joiner.on(NAMESPACE_ESCAPED_SEPARATOR);
private static final Splitter NAMESPACE_ESCAPED_SPLITTER =
Splitter.on(NAMESPACE_ESCAPED_SEPARATOR);
+ /**
+ * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link
+ * RESTUtil#namespaceToQueryParam(Namespace)}} instead.
+ */
+ @Deprecated public static final Joiner NAMESPACE_JOINER =
Joiner.on(NAMESPACE_SEPARATOR);
+
+ /**
+ * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link
+ * RESTUtil#namespaceFromQueryParam(String)} instead.
+ */
+ @Deprecated public static final Splitter NAMESPACE_SPLITTER =
Splitter.on(NAMESPACE_SEPARATOR);
+
private RESTUtil() {}
public static String stripTrailingSlash(String path) {
@@ -160,6 +170,41 @@ public class RESTUtil {
return URLDecoder.decode(encoded, StandardCharsets.UTF_8);
}
+ /**
+ * This converts the given namespace to a string and separates each part in
a multipart namespace
+ * using the unicode character '\u001f'. Note that this method is different
from {@link
+ * RESTUtil#encodeNamespace(Namespace)}, which uses the UTF-8 escaped
version of '\u001f', which
+ * is '0x1F'.
+ *
+ * <p>{@link #namespaceFromQueryParam(String)} should be used to convert the
namespace string back
+ * to a {@link Namespace} instance.
+ *
+ * @param namespace The namespace to convert
+ * @return The namespace converted to a string where each part in a
multipart namespace is
+ * separated using the unicode character '\u001f'
+ */
+ public static String namespaceToQueryParam(Namespace namespace) {
+ Preconditions.checkArgument(null != namespace, "Invalid namespace: null");
+ return RESTUtil.NAMESPACE_JOINER.join(namespace.levels());
+ }
+
+ /**
+ * This converts a namespace where each part in a multipart namespace has
been separated using
+ * '\u001f' to its original {@link Namespace} instance.
+ *
+ * <p>{@link #namespaceToQueryParam(Namespace)} should be used to convert
the {@link Namespace} to
+ * a string.
+ *
+ * @param namespace The namespace to convert
+ * @return The namespace instance from the given namespace string, where
each multipart separator
+ * ('\u001f') is converted to a separate namespace level
+ */
+ public static Namespace namespaceFromQueryParam(String namespace) {
+ Preconditions.checkArgument(null != namespace, "Invalid namespace: null");
+ return Namespace.of(
+
RESTUtil.NAMESPACE_SPLITTER.splitToStream(namespace).toArray(String[]::new));
+ }
+
/**
* Returns a String representation of a namespace that is suitable for use
in a URL / URI.
*
diff --git a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
index 35ef6dac35..675aa8ab6f 100644
--- a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
+++ b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
@@ -314,11 +314,7 @@ public class RESTCatalogAdapter extends BaseHTTPClient {
if (asNamespaceCatalog != null) {
Namespace ns;
if (vars.containsKey("parent")) {
- ns =
- Namespace.of(
- RESTUtil.NAMESPACE_SPLITTER
- .splitToStream(vars.get("parent"))
- .toArray(String[]::new));
+ ns = RESTUtil.namespaceFromQueryParam(vars.get("parent"));
} else {
ns = Namespace.empty();
}
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
index 6eb8f59bf6..fe022b3518 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
@@ -20,6 +20,7 @@ package org.apache.iceberg.rest;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Map;
import org.apache.iceberg.catalog.Namespace;
@@ -166,4 +167,49 @@ public class TestRESTUtil {
RESTUtil.resolveEndpoint("http://catalog-uri/";,
"relative-endpoint/refresh-endpoint"))
.isEqualTo("http://catalog-uri/relative-endpoint/refresh-endpoint";);
}
+
+ @Test
+ public void namespaceToQueryParam() {
+ assertThatThrownBy(() -> RESTUtil.namespaceToQueryParam(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid namespace: null");
+
+
assertThat(RESTUtil.namespaceToQueryParam(Namespace.empty())).isEqualTo("");
+ assertThat(RESTUtil.namespaceToQueryParam(Namespace.of(""))).isEqualTo("");
+
assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("ns"))).isEqualTo("ns");
+
+ // verify that the unicode character (\001f) and not its UTF-8 escaped
version (%1F) is used
+ assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one", "ns")))
+ .isEqualTo("one\u001fns")
+ .isNotEqualTo("one%1Fns");
+ assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one", "two",
"ns")))
+ .isEqualTo("one\u001ftwo\u001fns")
+ .isNotEqualTo("one%1Ftwo%1Fns");
+ assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one.two",
"three.four", "ns")))
+ .isEqualTo("one.two\u001fthree.four\u001fns")
+ .isNotEqualTo("one.two%1Fthree.four%1Fns");
+ }
+
+ @Test
+ public void namespaceFromQueryParam() {
+ assertThatThrownBy(() -> RESTUtil.namespaceFromQueryParam(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid namespace: null");
+
+
assertThat(RESTUtil.namespaceFromQueryParam("")).isEqualTo(Namespace.of(""));
+
assertThat(RESTUtil.namespaceFromQueryParam("ns")).isEqualTo(Namespace.of("ns"));
+ assertThat(RESTUtil.namespaceFromQueryParam("one\u001fns"))
+ .isEqualTo(Namespace.of("one", "ns"));
+ assertThat(RESTUtil.namespaceFromQueryParam("one\u001ftwo\u001fns"))
+ .isEqualTo(Namespace.of("one", "two", "ns"));
+
assertThat(RESTUtil.namespaceFromQueryParam("one.two\u001fthree.four\u001fns"))
+ .isEqualTo(Namespace.of("one.two", "three.four", "ns"));
+
+ // using the UTF-8 escaped version will produce a wrong namespace instance
+
assertThat(RESTUtil.namespaceFromQueryParam("one%1Fns")).isEqualTo(Namespace.of("one%1Fns"));
+ assertThat(RESTUtil.namespaceFromQueryParam("one%1Ftwo%1Fns"))
+ .isEqualTo(Namespace.of("one%1Ftwo%1Fns"));
+ assertThat(RESTUtil.namespaceFromQueryParam("one%1Ftwo\u001fns"))
+ .isEqualTo(Namespace.of("one%1Ftwo", "ns"));
+ }
}
