This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new a5c6bc9d2 fix(io): skip subdirectory markers in blobfs WalkDir (#2187)
a5c6bc9d2 is described below
commit a5c6bc9d26bbaaaea25af025e4012ebd4d958d87
Author: Revanth <[email protected]>
AuthorDate: Mon Oct 19 10:21:40 2026 -0500
fix(io): skip subdirectory markers in blobfs WalkDir (#2187)
---
io/gocloud/blobfs/blob.go | 39 ++++++++++----------------------------
io/gocloud/blobfs/blob_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/io/gocloud/blobfs/blob.go b/io/gocloud/blobfs/blob.go
index 5988870aa..19a6474b4 100644
--- a/io/gocloud/blobfs/blob.go
+++ b/io/gocloud/blobfs/blob.go
@@ -409,26 +409,6 @@ func walkedURIPath(location ObjectLocation, walked string)
string {
return location.uriPrefix + walked
}
-// isDirectoryMarker reports whether dirEntry is the marker object for the
-// path passed to WalkDir
-func isDirectoryMarker(walkRootKey string, dirEntry fs.DirEntry) bool {
- // Directory entries and nil entries are never the marker object itself.
- if dirEntry == nil || dirEntry.IsDir() {
- return false
- }
-
- info, err := dirEntry.Info()
- if err != nil {
- return false
- }
-
- // blob fs is the only implementation that uses directory markers
- // so we can use a type assertion to check
- obj, ok := info.Sys().(*blob.ListObject)
-
- return ok && obj.Key == directoryMarker(walkRootKey)
-}
-
// walkFS walks a bucket for fs.WalkDir, treating any key with children as a
directory,
// so the 0-byte directory blobs of ADLS hierarchical namespaces (OneLake)
aren't files.
type walkFS struct{ bfs *FileIO }
@@ -484,6 +464,7 @@ func (w walkFS) ReadDir(name string) ([]fs.DirEntry, error)
{
var files []*blob.ListObject
dirs := map[string]bool{}
+ hasMarker := false
iter := w.bfs.List(&blob.ListOptions{Prefix: prefix, Delimiter: "/"})
for {
obj, err := iter.Next(w.bfs.ctx)
@@ -494,6 +475,13 @@ func (w walkFS) ReadDir(name string) ([]fs.DirEntry,
error) {
return nil, blobErrToFsErr("readdir", name, err)
}
+ if obj.Key == prefix {
+ // The "name/" marker of the directory being read, not
a child of it.
+ hasMarker = true
+
+ continue
+ }
+
if obj.IsDir {
dirs[strings.TrimSuffix(strings.TrimPrefix(obj.Key,
prefix), "/")] = true
} else {
@@ -513,8 +501,6 @@ func (w walkFS) ReadDir(name string) ([]fs.DirEntry, error)
{
continue
}
- // Name entries the way gocloud's fs.FS does, so a "dir/"
marker keeps
- // the name it had before walkFS.
entries = append(entries, walkFileEntry{blobFileInfo{
name: pathpkg.Base(obj.Key),
size: obj.Size,
@@ -524,7 +510,8 @@ func (w walkFS) ReadDir(name string) ([]fs.DirEntry, error)
{
}})
}
- if len(entries) == 0 && name != "." {
+ // A directory holding only its marker is empty, not missing.
+ if len(entries) == 0 && name != "." && !hasMarker {
return nil, &fs.PathError{Op: "readdir", Path: name, Err:
fs.ErrNotExist}
}
@@ -567,12 +554,6 @@ func (bfs *FileIO) WalkDir(root string, fn fs.WalkDirFunc)
error {
}
return fs.WalkDir(walkFS{bfs}, walkPath, func(path string, d
fs.DirEntry, err error) error {
- // Skip the marker for the path passed to WalkDir. Otherwise
walking
- // "warehouse/ns" reports "warehouse/ns/" as a child file.
- if err == nil && isDirectoryMarker(walkPath, d) {
- return nil
- }
-
if location.hasAuthority {
path = walkedURIPath(location, path)
}
diff --git a/io/gocloud/blobfs/blob_test.go b/io/gocloud/blobfs/blob_test.go
index 125a2f042..272833207 100644
--- a/io/gocloud/blobfs/blob_test.go
+++ b/io/gocloud/blobfs/blob_test.go
@@ -893,6 +893,49 @@ func TestBlobFileIOWalkDirSkipsDirectoryMarker(t
*testing.T) {
assert.Equal(t, []string{"s3://test-bucket/warehouse/ns"}, paths)
}
+func TestBlobFileIOWalkDirSkipsSubdirectoryMarkers(t *testing.T) {
+ ctx := context.Background()
+
+ bucket := memblob.OpenBucket(nil)
+ defer bucket.Close()
+
+ // "tbl/empty/" is a marker with no children, so it is an empty
directory.
+ for _, k := range []string{"tbl/", "tbl/data/", "tbl/empty/"} {
+ require.NoError(t, bucket.WriteAll(ctx, k, nil, nil))
+ }
+ require.NoError(t, bucket.WriteAll(ctx, "tbl/data/a.parquet",
[]byte("a"), nil))
+ require.NoError(t, bucket.WriteAll(ctx,
"tbl/metadata/v1.metadata.json", []byte("b"), nil))
+
+ bfs := testBlobFileIO(ctx, "mybucket", bucket)
+
+ var dirs, files []string
+ err := bfs.WalkDir("s3://mybucket/tbl", func(path string, d
fs.DirEntry, err error) error {
+ if err != nil {
+ return err
+ }
+
+ if d.IsDir() {
+ dirs = append(dirs, path)
+ } else {
+ files = append(files, path)
+ }
+
+ return nil
+ })
+ require.NoError(t, err)
+
+ assert.Equal(t, []string{
+ "s3://mybucket/tbl",
+ "s3://mybucket/tbl/data",
+ "s3://mybucket/tbl/empty",
+ "s3://mybucket/tbl/metadata",
+ }, dirs)
+ assert.Equal(t, []string{
+ "s3://mybucket/tbl/data/a.parquet",
+ "s3://mybucket/tbl/metadata/v1.metadata.json",
+ }, files)
+}
+
func TestBlobFileIORemoveAll(t *testing.T) {
ctx := context.Background()
bucket := memblob.OpenBucket(nil)
