You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/08/13 12:00:35 UTC
[PR] perf(table): flatten orphan URI equivalences [iceberg-go]
fallintoplace opened a new pull request, #1810:
URL: https://github.com/apache/iceberg-go/pull/1810
Closes #1808
## What changed
- Expand comma-separated scheme and authority groups once when options are applied.
- Use direct map lookups during orphan path normalization and prefix checks.
- Make overlapping groups deterministic. The lexicographically last group wins.
- Add tests and a benchmark across path and group counts.
## Why
Orphan cleanup applies URI equivalence rules to every referenced and listed path. The old code scanned and split every configured group for each lookup. This changes that to one lookup.
The impact is speed x amount: lookup savings are multiplied by the number of paths compared.
## Checks
- `go test ./...`
- `go test ./table`
- `go test ./table -run '^$' -bench 'BenchmarkApplyURIEquivalence$' -benchmem -count=1`\n- `go vet ./table`
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): flatten orphan URI equivalences [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #1810:
URL: https://github.com/apache/iceberg-go/pull/1810#issuecomment-5319205355
Since this is a draft, i'll hold off on further review until it is marked ready
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): flatten orphan URI equivalences [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1810:
URL: https://github.com/apache/iceberg-go/pull/1810#discussion_r3798464088
##########
table/orphan_cleanup_test.go:
##########
@@ -86,6 +86,78 @@ func TestOrphanCleanupOptions(t *testing.T) {
assert.Equal(t, authorities, cfg.equalAuthorities)
}
+func TestFlattenURIEquivalences(t *testing.T) {
+ equivalences := map[string]string{
+ "s3,s3a,s3n": "s3",
+ "s3a,gs": "gs",
+ "single": "canonical",
+ }
+
+ assert.Equal(t, map[string]string{
+ "s3": "s3",
+ "s3a": "gs",
+ "s3n": "s3",
+ "gs": "gs",
+ "single": "canonical",
+ }, flattenURIEquivalences(equivalences))
+}
+
+func TestFlattenURIEquivalencesUsesLexicographicallyLastGroup(t *testing.T) {
+ equivalences := map[string]string{
+ "s3, s3a": "s3",
+ "s3a,s3n": "s3n",
+ }
+
+ assert.Equal(t, map[string]string{
+ "s3": "s3",
+ "s3a": "s3n",
+ "s3n": "s3n",
+ }, flattenURIEquivalences(equivalences))
+}
+
+func TestFlattenURIEquivalencesPreservesExactMappingPrecedence(t *testing.T) {
+ equivalences := map[string]string{
+ "host1": "canonical",
+ "host1,host2": "other",
+ "host2": "canonical",
+ }
+
+ flattened := flattenURIEquivalences(equivalences)
+ assert.Equal(t, "canonical", flattened["host1"])
+ assert.Equal(t, "canonical", flattened["host2"])
+}
+
+func TestEqualAuthoritiesPreservesExactMappingAcrossOptions(t *testing.T) {
+ cfg := newOrphanCleanupConfig(
+ WithEqualAuthorities(map[string]string{
+ "host1": "canonical",
+ }),
+ WithEqualAuthorities(map[string]string{
+ "host1,host2": "other",
+ "host2": "canonical",
+ }),
+ )
+
+ assert.Equal(t, "canonical", applyAuthorityEquivalence("host1", cfg.equalAuthorities))
+ assert.Equal(t, "canonical", applyAuthorityEquivalence("host2", cfg.equalAuthorities))
+}
+
+func TestNewOrphanCleanupConfigFlattensURIEquivalences(t *testing.T) {
+ cfg := newOrphanCleanupConfig(
+ WithEqualSchemes(map[string]string{
+ "s3,s3a": "s3",
+ }),
+ WithEqualAuthorities(map[string]string{
+ "host1,host2": "canonical",
Review Comment:
Could you make this exercise the required ADLS behavior through `normalizeURLPath` (and ideally `checkPrefixMismatch`) using `abfs://container@account-*.dfs.core.windows.net/...`, rather than only calling `applyAuthorityEquivalence` directly? That regression test currently fails because `net/url` removes `container@` from `URL.Host`, which is the gap hidden by these host-only values.
##########
table/orphan_cleanup.go:
##########
@@ -888,24 +917,10 @@ func applySchemeEquivalence(scheme string, equalSchemes map[string]string) strin
// Based on Apache Iceberg Java's equalAuthorities logic (lines 546, 161-165, 392-403).
// https://github.com/apache/iceberg/blob/07c088fce9c54369864dcb6da16006e78206048b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java#L1
func applyAuthorityEquivalence(authority string, equalAuthorities map[string]string) string {
- if equalAuthorities == nil {
- return authority
- }
-
if canonical, exists := equalAuthorities[authority]; exists {
Review Comment:
**Blocking:** `normalizeURLPath` and `checkPrefixMismatch` pass `parsedURL.Host` here, but Go's `net/url` separates `abfs://container@host/...` into `User=container` and `Host=host`. As a result, flattened keys such as `container@account-a.dfs.core.windows.net` never match, and `normalizeURLPath` also reconstructs the URL without `User`, dropping the container. A direct probe with two configured `container@host` authorities normalized them to different URLs (`abfs://account-a...` vs `abfs://account-b...`). Please perform authority lookup on the complete authority (`User@Host`) and preserve/reconstruct user-info when normalizing; the same helper should be used by the prefix-mismatch path.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): flatten orphan URI equivalences [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1810:
URL: https://github.com/apache/iceberg-go/pull/1810
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org