You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "venkateshwaracholan (via GitHub)" <gi...@apache.org> on 2026/05/29 17:00:14 UTC

[PR] Add support for unique-table-location [polaris]

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

   Fixes #4492
   
   ## Summary
   
   Add support for Iceberg's `unique-table-location` catalog property when creating tables without an explicit location.
   
   ## Changes
   
   * Honor `CatalogProperties.UNIQUE_TABLE_LOCATION` when computing default table locations.
   * Use Iceberg's `LocationUtil.tableLocation(...)` helper to generate table directory names.
   * Re-enable the previously disabled Iceberg compatibility test `createTableInUniqueLocation()`.
   * Add a changelog entry.
   
   ## Validation
   
   Verified Iceberg 1.11 uses the same `LocationUtil.tableLocation(...)` helper when `unique-table-location` is enabled.
   
   Ran:
   
   ```bash
   ./gradlew :polaris-runtime-service:test \
     --tests "org.apache.polaris.service.catalog.iceberg.IcebergCatalogRelationalTest.createTableInUniqueLocation"
   ```
   
   Result: passed.
   
   Also ran:
   
   ```bash
   ./gradlew :polaris-runtime-service:spotlessJavaCheck
   ./gradlew compileAll
   ```
   
   Both passed.
   
   
   ## 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] Add support for unique-table-location [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4593774768

   I don’t think this implementation is correct for Polaris.
   It appears to satisfy the inherited Iceberg `createTableInUniqueLocation` test, but it does not trace all of Polaris’s create-location paths.
   
   The main issue is that `defaultWarehouseLocation(...)` and `buildPrefixedLocation(...)` are shared by more than "direct table create without an explicit location":
   
   - Direct table create calls `buildTable(...).withLocation(request.location()).create()`. When `request.location()` is `null`, the Polaris table builder can route through `transformTableLikeLocation(...)`, and depending on prefix config may use `buildPrefixedLocation(...)` or `defaultWarehouseLocation(...)`.
   - Staged table create without a location uses `buildTable(...).createTransaction().table().location()` in `IcebergCatalogHandler.stageTableCreateHelper(...)`. That does not call `withLocation(null)`, so it bypasses the same transform path used by direct create. This means direct create and staged create can derive different locations under `DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED`.
   - Views also use the same catalog default-location machinery. `createView(...)` eventually calls `.withLocation(request.location())`, and the Polaris view builder also calls `transformTableLikeLocation(...)`. If `buildPrefixedLocation(...)` or `defaultWarehouseLocation(...)` blindly uses `LocationUtil.tableLocation(...)`, then views can inherit `unique-table-location` and get a `viewName-uuid` suffix, which is not what Iceberg’s table-specific `unique-table-location` property should mean.
   
   I think the fix needs a table-specific helper for deriving default table create locations, and both direct create and staged create should use that helper when the request has no explicit location. View default locations should stay view-name-based, and namespace/register/load/drop/rename paths should not be affected.
   
   The fix also need targeted tests for those changes.


-- 
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] Add support for unique-table-location [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4624932573

   Dev mailing discussion: https://lists.apache.org/thread/f72f0k6g6kco0sokvgqnn5dpjdc1xl92


-- 
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] Add support for unique-table-location [polaris]

Posted by "venkateshwaracholan (via GitHub)" <gi...@apache.org>.
venkateshwaracholan closed pull request #4582: Add support for unique-table-location
URL: https://github.com/apache/polaris/pull/4582


-- 
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] Add support for unique-table-location [polaris]

Posted by "venkateshwaracholan (via GitHub)" <gi...@apache.org>.
venkateshwaracholan commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4601163321

   > I don’t think this implementation is correct for Polaris. It appears to satisfy the inherited Iceberg `createTableInUniqueLocation` test, but it does not trace all of Polaris’s create-location paths.
   > 
   > The main issue is that `defaultWarehouseLocation(...)` and `buildPrefixedLocation(...)` are shared by more than "direct table create without an explicit location":
   > 
   > * Direct table create calls `buildTable(...).withLocation(request.location()).create()`. When `request.location()` is `null`, the Polaris table builder can route through `transformTableLikeLocation(...)`, and depending on prefix config may use `buildPrefixedLocation(...)` or `defaultWarehouseLocation(...)`.
   > * Staged table create without a location uses `buildTable(...).createTransaction().table().location()` in `IcebergCatalogHandler.stageTableCreateHelper(...)`. That does not call `withLocation(null)`, so it bypasses the same transform path used by direct create. This means direct create and staged create can derive different locations under `DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED`.
   > * Views also use the same catalog default-location machinery. `createView(...)` eventually calls `.withLocation(request.location())`, and the Polaris view builder also calls `transformTableLikeLocation(...)`. If `buildPrefixedLocation(...)` or `defaultWarehouseLocation(...)` blindly uses `LocationUtil.tableLocation(...)`, then views can inherit `unique-table-location` and get a `viewName-uuid` suffix, which is not what Iceberg’s table-specific `unique-table-location` property should mean.
   > 
   > I think the fix needs a table-specific helper for deriving default table create locations, and both direct create and staged create should use that helper when the request has no explicit location. View default locations should stay view-name-based, and namespace/register/load/drop/rename paths should not be affected.
   > 
   > The fix also need targeted tests for those changes.
   
   Thanks for the detailed review — it helped highlight the Polaris-specific create-location paths that weren't covered by the Iceberg compatibility test.
   
   I've pushed an update that addresses the staged create and view-location concerns, and added targeted tests for both scenarios. I also updated the location derivation logic so direct and staged table creates follow the same path when unique-table-location is enabled.
   


-- 
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] Add support for unique-table-location [polaris]

Posted by "venkateshwaracholan (via GitHub)" <gi...@apache.org>.
venkateshwaracholan commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4918007629

   Closing this in favour of #4492 


-- 
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] Add support for unique-table-location [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4624494633

   > We should probably sync up with #4606 CC: @flyrain
   > 
   > I suggest starting a new ML thread.
   
   @dimas-b, thanks for tagging me here. I believe this is related. We need to think it through. Will send out a dev mail soon.


-- 
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] Add support for unique-table-location [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4582:
URL: https://github.com/apache/polaris/pull/4582#issuecomment-4624387078

   We should probably sync up with #4606 CC: @flyrain 
   
   I suggest starting a new ML thread.


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