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
`[email protected]` 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
