anxkhn opened a new pull request, #4975: URL: https://github.com/apache/polaris/pull/4975
`buildPrefixedLocation` in `LocalIcebergCatalog` derives a table's durable default storage location by percent-encoding each namespace level and the table name with `URLEncoder.encode(..., Charset.defaultCharset())`. Using the JVM's *default* charset means that, for a namespace or table whose name contains non-ASCII characters, the persisted location depends on the platform default charset of whatever JVM created the table. On a non-UTF-8 JVM the same logical table resolves to a different (or lossy) location than on a UTF-8 JVM, which breaks portability and the round-trip expectation that a table's location is stable and deterministic. This encodes both segments with `StandardCharsets.UTF_8`. That matches the canonical namespace codec `PolarisEntityUtils.encodeNamespace` (which already uses `StandardCharsets.UTF_8`) and every other `URLEncoder`/`URLDecoder` call site in the service, so the derived location is now deterministic and charset-independent. These two call sites were the only ones still using the platform default charset. Current behavior: with object-storage prefixing enabled (`DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED`), the hashed default location for a table under a non-ASCII namespace/table name is percent-encoded using the JVM default charset, so it is not stable across deployments with different default charsets. Expected behavior after this change: the derived location is always the UTF-8 percent-encoded form, independent of the JVM default charset. Added a regression test (`IcebergOverlappingTableTest`, `testHashedTableLocationsEncodeNonAsciiIdentifiersAsUtf8`) that creates a catalog, namespace, and table using non-ASCII identifiers with object-storage prefixing enabled and asserts the derived location ends with the UTF-8 percent-encoded `namespace/table` suffix and never contains the raw identifiers. The two small private helper overloads it uses delegate from the existing helper signatures, so all existing callers are untouched. ### Checklist - [ ] Don't disclose security issues! (contact [email protected]) - N/A, not a security issue. - [ ] Clearly explained why the changes are needed, or linked related issues - see above (no matching issue; self-found correctness bug, so no `Fixes #` line). - [x] Added/updated tests with good coverage, or manually tested (and explained how) - new regression test in `IcebergOverlappingTableTest`. - [ ] Added comments for complex logic - N/A (one-line encoder change; the test carries an explanatory comment). - [x] Updated `CHANGELOG.md` - added a `### Fixes` entry under `[Unreleased]`. - [ ] Updated documentation in `site/content/in-dev/unreleased` - N/A (no user-facing config/API surface changes; only the encoding of a derived internal path). > Note when actually filling the template on GitHub: check the boxes that apply and > leave the N/A ones per the notes above. Do not paste these parenthetical notes into > the PR verbatim - they are guidance for Anas. --- ## Optional AI-assistance disclosure (Anas's call) Polaris CONTRIBUTING.md ("Guidelines for AI-assisted Contributions") *encourages* (does not require) disclosing significant AI assistance, and is explicit that the human submitter is the author and must understand and be able to justify the change end-to-end in review. This diff is small and the rationale is fully explained above, so a one-line disclosure is low-cost and honest. Suggested line to append to the PR body if you want it: ``` Prepared with AI assistance; I have reviewed the change and take responsibility for it. ``` Include it or not at your discretion - just be ready to explain the encoding fix and the test if a reviewer asks. --- ## Commit Single commit, Conventional Commits style matching repo history, no DCO sign-off (correct: ASF uses the ICLA; the history has zero `Signed-off-by`). Message body already explains why + what. No `Fixes #`/`Related to #` line because there is no matching issue. ``` fix(catalog): encode default table locations with UTF-8 buildPrefixedLocation derived a table's default storage location by percent-encoding the namespace levels and table name with URLEncoder.encode(..., Charset.defaultCharset()). For non-ASCII identifiers the persisted location then depended on the JVM's default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM, breaking portability and round-trip expectations. Encode both segments with StandardCharsets.UTF_8, matching PolarisEntityUtils.encodeNamespace and every other URLEncoder call site in the service. Add a regression test that creates a table under non-ASCII namespace and table identifiers with object-storage prefixing enabled and asserts the derived location uses the UTF-8 percent-encoded form regardless of the platform default charset. ``` --- ## Files changed (3) ``` CHANGELOG.md | 1 + runtime/service/.../catalog/iceberg/LocalIcebergCatalog.java | 6 +- (import swap + 2 encoder args) runtime/service/src/test/.../catalog/iceberg/IcebergOverlappingTableTest.java | 69 ++ (new test + 2 helper overloads) ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
