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

Posted to issues@polaris.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/08/31 23:00:48 UTC

[PR] Skip the default-location check when the namespace location was not requested [polaris]

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

   `createNamespaceInternal` validates the namespace location against the default derived from its parent even when the request specified no location. In that case the location being checked is the one Polaris just derived: `setBaseLocation` stores it in the entity's properties, so the validator reads that value back and compares it against the same derivation, and it can only pass. `setProperties` already guards the same call on whether the request carried a location, so this makes create consistent with it.
   
   No behaviour change. Added tests for sanity which covers the four location cases - no location, the default stated explicitly, a different location, and a different location with `ALLOW_NAMESPACE_CUSTOM_LOCATION` enabled - and passes with and without the guard.
   
   This fixes the follow-up suggestion along with test covering all the 4 cases from @flyrain here: https://github.com/apache/polaris/pull/5357#issuecomment-5481307673
   
   ## 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] Skip the default-location check when the namespace location was not requested [polaris]

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

Copilot commented on code in PR #5422:
URL: https://github.com/apache/polaris/pull/5422#discussion_r3899192669


##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java:
##########
@@ -730,6 +730,67 @@ void testCreateNamespaceWhenBaseLocationIsNestedInsideAllowedLocation(@TempDir P
         .hasMessageContaining("has a custom location");
   }
 
+  /**
+   * The four namespace-location cases, with ALLOW_NAMESPACE_CUSTOM_LOCATION off for the first three
+   * and on for the last.
+   */
+  @Test
+  void testNamespaceCustomLocationRules(@TempDir Path tmpDir) {
+    Map<String, Object> baseConfig =
+        Map.of(
+            "ALLOW_INSECURE_STORAGE_TYPES",
+            "true",
+            "SUPPORTED_CATALOG_STORAGE_TYPES",
+            List.of("FILE"));
+    String catalogLocation = tmpDir.toAbsolutePath().toUri().toString();
+
+    TestServices strict = TestServices.builder().config(baseConfig).build();
+    createCatalog(strict, Map.of(), catalogLocation, List.of(catalogLocation));
+    String defaultBase = String.format("%s/%s", catalogLocation, catalog);
+
+    // 1. No location requested: the default is derived and used.
+    assertThat(createNamespaceStatus(strict, "ns1", null))
+        .isEqualTo(Response.Status.OK.getStatusCode());
+
+    // 2. The default location, stated explicitly: allowed, it is explicit but not custom.
+    assertThat(createNamespaceStatus(strict, "ns2", defaultBase + "/ns2"))
+        .isEqualTo(Response.Status.OK.getStatusCode());
+
+    // 3. A location other than the default: rejected as custom.
+    assertThatThrownBy(() -> createNamespaceStatus(strict, "ns3", catalogLocation + "/elsewhere"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("has a custom location");

Review Comment:
   `Path.toUri().toString()` for a directory ends with a trailing `/`, so `catalogLocation + "/elsewhere"` will produce a double-slash path (`...//elsewhere`). That makes the test data less clear and can interact with location normalization. Consider using the same concatenation style as the earlier test in this file (`base + "elsewhere"`).
   
   This issue also appears on line 768 of the same file.



-- 
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] Skip the default-location check when the namespace location was not requested [polaris]

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

flyrain merged PR #5422:
URL: https://github.com/apache/polaris/pull/5422


-- 
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