You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2026/02/09 16:57:06 UTC

[PR] [Iceberg 1.11] Replace deprecated RESTUtil.encodeNamespace(String) [polaris]

adutra opened a new pull request, #3713:
URL: https://github.com/apache/polaris/pull/3713

   <!--
   📝 Describe what changes you're proposing, especially breaking or user-facing changes. 
   📖 See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
   -->
   
   ## Checklist
   - [ ] 🛡️ Don't disclose security issues! (contact security@apache.org)
   - [ ] 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
   - [ ] 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
   - [ ] 💡 Added comments for complex logic
   - [ ] 🧾 Updated `CHANGELOG.md` (if needed)
   - [ ] 📚 Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.

snazy commented on PR #3713:
URL: https://github.com/apache/polaris/pull/3713#issuecomment-3879973234

   > > We should also guard against any change to the separator.
   > 
   > The separator is declared as a constant in `NamespaceEntity` and there is currently no way to specify a different separator. Do you think we need more safeguards?
   
   I _think_ there's an Iceberg config property for this, which could leak into Polaris. Just wanna be sure that such situations cannot happen.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.

adutra commented on PR #3713:
URL: https://github.com/apache/polaris/pull/3713#issuecomment-4006497616

   Rebased to fix conflicts.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.

adutra commented on PR #3713:
URL: https://github.com/apache/polaris/pull/3713#issuecomment-4049048186

   > I _think_ there's an Iceberg config property for this, which could leak into Polaris. Just wanna be sure that such situations cannot happen.
   
   You are right: there is a new field `RESTCatalogProperties.NAMESPACE_SEPARATOR`. I added this property to the /config endpoint as an override.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.

adutra commented on code in PR #3713:
URL: https://github.com/apache/polaris/pull/3713#discussion_r2942090820


##########
polaris-core/src/main/java/org/apache/polaris/core/entity/NamespaceEntity.java:
##########
@@ -33,6 +33,15 @@ public class NamespaceEntity extends PolarisEntity implements LocationBasedEntit
   // RESTUtil-encoded parent namespace.
   public static final String PARENT_NAMESPACE_KEY = "parent-namespace";
 
+  /**
+   * The URL-encoded UTF-8 representation of the namespace separator character (0x1F).
+   *
+   * <p>This constant mirrors the {@code
+   * org.apache.iceberg.rest.RESTUtil#NAMESPACE_SEPARATOR_URLENCODED_UTF_8} constant which is not

Review Comment:
   It's not deprecated, but unfortunately it's not public either. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.

adutra commented on code in PR #3713:
URL: https://github.com/apache/polaris/pull/3713#discussion_r2942104492


##########
polaris-core/src/main/java/org/apache/polaris/core/entity/NamespaceEntity.java:
##########
@@ -33,6 +33,15 @@ public class NamespaceEntity extends PolarisEntity implements LocationBasedEntit
   // RESTUtil-encoded parent namespace.
   public static final String PARENT_NAMESPACE_KEY = "parent-namespace";
 
+  /**
+   * The URL-encoded UTF-8 representation of the namespace separator character (0x1F).
+   *
+   * <p>This constant mirrors the {@code
+   * org.apache.iceberg.rest.RESTUtil#NAMESPACE_SEPARATOR_URLENCODED_UTF_8} constant which is not
+   * public.
+   */
+  public static final String NAMESPACE_SEPARATOR_URLENCODED_UTF_8 = "%1F";

Review Comment:
   I personally think it's not possible/desirable.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on code in PR #3713:
URL: https://github.com/apache/polaris/pull/3713#discussion_r2941881062


##########
polaris-core/src/main/java/org/apache/polaris/core/entity/NamespaceEntity.java:
##########
@@ -33,6 +33,15 @@ public class NamespaceEntity extends PolarisEntity implements LocationBasedEntit
   // RESTUtil-encoded parent namespace.
   public static final String PARENT_NAMESPACE_KEY = "parent-namespace";
 
+  /**
+   * The URL-encoded UTF-8 representation of the namespace separator character (0x1F).
+   *
+   * <p>This constant mirrors the {@code
+   * org.apache.iceberg.rest.RESTUtil#NAMESPACE_SEPARATOR_URLENCODED_UTF_8} constant which is not

Review Comment:
   Is `org.apache.iceberg.rest.RESTUtil#NAMESPACE_SEPARATOR_URLENCODED_UTF_8` deprecated in the Iceberg repo as well? If not, we could reuse it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on code in PR #3713:
URL: https://github.com/apache/polaris/pull/3713#discussion_r2941892838


##########
polaris-core/src/main/java/org/apache/polaris/core/entity/NamespaceEntity.java:
##########
@@ -33,6 +33,15 @@ public class NamespaceEntity extends PolarisEntity implements LocationBasedEntit
   // RESTUtil-encoded parent namespace.
   public static final String PARENT_NAMESPACE_KEY = "parent-namespace";
 
+  /**
+   * The URL-encoded UTF-8 representation of the namespace separator character (0x1F).
+   *
+   * <p>This constant mirrors the {@code
+   * org.apache.iceberg.rest.RESTUtil#NAMESPACE_SEPARATOR_URLENCODED_UTF_8} constant which is not
+   * public.
+   */
+  public static final String NAMESPACE_SEPARATOR_URLENCODED_UTF_8 = "%1F";

Review Comment:
   Are we going to support configurable separator in the future?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Iceberg 1.11] Replace deprecated encodeNamespace and decodeNamespace methods [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.

adutra commented on PR #3713:
URL: https://github.com/apache/polaris/pull/3713#issuecomment-3879458863

   > We should also guard against any change to the separator.
   
   The separator is declared as a constant in `NamespaceEntity` and there is currently no way to specify a different separator. Do you think we need more safeguards?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org