From 5437153f3994db64e4292f36a131e2a3ba5174f8 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:07:01 +0900 Subject: [PATCH 1/6] Cache each explicit version of an object separately in info() info(path, version_id=...) looked up and stored the HeadObject result under the path without the version, so a later lookup of another version, or of the path itself, got the first version's metadata. Look up an explicit version under its version-qualified path instead, and compare cached entries by their name without the version suffix, so a second info() of a ?versionId= path no longer returns a directory. Fixes #932 Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 13 ++++++++++--- tests/pyathena/filesystem/test_s3.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index ba7abb984..17849172f 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -585,7 +585,8 @@ def info(self, path: str, **kwargs) -> S3Object: exists, with a ListObjectsV2 request (``Delimiter="/"``, ``MaxKeys=1``) that checks whether it is a key prefix; a bucket path is looked up with HeadBucket. With ``version_aware``, a cached file - entry without a version ID is looked up again. + entry without a version ID is looked up again. An explicit version is + cached under its version-qualified path, apart from other versions. Args: path: S3 path (e.g., "s3://bucket" or "s3://bucket/key"). @@ -603,6 +604,12 @@ def info(self, path: str, **kwargs) -> S3Object: path = self._strip_protocol(path) bucket, key, path_version_id = self.parse_path(path) version_id = path_version_id if path_version_id else kwargs.pop("version_id", None) + if key and version_id and not path_version_id: + # Look up an explicit version under its version-qualified path so + # that it neither reuses nor replaces the entry of another version. + path = f"{path}?versionId={version_id}" + # Cached entries are named without the version suffix. + name = path.split("?", 1)[0] if path in ["/", ""]: return S3Object( init={ @@ -621,8 +628,8 @@ def info(self, path: str, **kwargs) -> S3Object: caches: list[S3Object] | S3Object | None = self._ls_from_cache(path) if caches is not None: if isinstance(caches, list): - cache = next((c for c in caches if c.name == path), None) - elif caches.name == path: + cache = next((c for c in caches if c.name == name), None) + elif caches.name == name: cache = caches else: cache = None diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 2d0fa6cc0..093b441cb 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -504,6 +504,25 @@ def test_head_object_version_aware(self): fs._call.return_value = {"ContentLength": 4, "ETag": '"etag"', "VersionId": "v1"} assert fs._head_object("bucket/key").version_id == "v1" + @pytest.mark.parametrize("version_aware", [False, True]) + def test_info_caches_each_version_separately(self, version_aware): + fs = self._make_fs() + fs.version_aware = version_aware + responses = { + None: {"ContentLength": 3, "ETag": '"e3"', "VersionId": "v3"}, + "v1": {"ContentLength": 1, "ETag": '"e1"', "VersionId": "v1"}, + "v2": {"ContentLength": 2, "ETag": '"e2"', "VersionId": "v2"}, + } + fs._call.side_effect = lambda _, **kwargs: responses[kwargs.get("VersionId")] + + for _ in range(2): + assert fs.info("s3://bucket/key", version_id="v1").size == 1 + assert fs.info("s3://bucket/key", version_id="v2").size == 2 + assert fs.info("s3://bucket/key?versionId=v1").size == 1 + assert fs.info("s3://bucket/key").size == 3 + # The second round is served from the cache. + assert fs._call.call_count == 3 + def test_object_version_info_paginates(self): fs = self._make_fs() fs._call.side_effect = [ From 9324a6432696929b51b3a80d2c9886cab72a2b07 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:09:35 +0900 Subject: [PATCH 2/6] Read the same version of an object twice in the filesystem tests The second open of a ?versionId= path read the metadata cached by the first one as a directory and returned no data. Co-Authored-By: Claude Opus 5.5 --- tests/pyathena/filesystem/test_s3.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 093b441cb..7ba9ecd89 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -1703,6 +1703,14 @@ def test_version_aware_read(self, fs): with fs.open(path, "rb") as f: assert f.read() == b"0123456789" + def test_read_version_twice(self, fs): + # An unversioned bucket stores each object as the "null" version. + path = f"s3://{ENV.s3_staging_bucket}/{ENV.s3_filesystem_test_file_key}?versionId=null" + # The second open reads the metadata cached by the first one. + for _ in range(2): + with fs.open(path, "rb") as f: + assert f.read() == b"0123456789" + def test_file_url_metadata_getxattr_setxattr(self, fs): path = ( f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/" From db4970e24e02f721164d2e2eaa0cee25affc20ab Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:12:36 +0900 Subject: [PATCH 3/6] Look up the null version of an object every time Writes invalidate only the path without the version, and overwriting an object in a bucket without versioning replaces its null version. A cached null version would keep the old ETag, and the next read with IfMatch would fail with PreconditionFailed. Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 6 +++++- tests/pyathena/filesystem/test_s3.py | 29 +++++++++++++++++++++------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index 17849172f..86d839eaa 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -366,7 +366,11 @@ def _head_object( key=key, version_id=version_id, ) - self.dircache[path] = file + # Writes invalidate only the path without the version, and an + # overwrite replaces the "null" version of a bucket without + # versioning, so that version is looked up every time. + if path_version_id != "null": + self.dircache[path] = file else: file = self.dircache[path] return file diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 7ba9ecd89..b1b13fc2e 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -523,6 +523,16 @@ def test_info_caches_each_version_separately(self, version_aware): # The second round is served from the cache. assert fs._call.call_count == 3 + def test_info_does_not_cache_null_version(self): + fs = self._make_fs() + fs._call.return_value = {"ContentLength": 4, "ETag": '"etag"', "VersionId": "null"} + + for _ in range(2): + assert fs.info("s3://bucket/key", version_id="null").size == 4 + assert fs.info("s3://bucket/key?versionId=null").size == 4 + # An overwrite can replace the null version, so it is looked up every time. + assert fs._call.call_count == 4 + def test_object_version_info_paginates(self): fs = self._make_fs() fs._call.side_effect = [ @@ -1703,13 +1713,18 @@ def test_version_aware_read(self, fs): with fs.open(path, "rb") as f: assert f.read() == b"0123456789" - def test_read_version_twice(self, fs): - # An unversioned bucket stores each object as the "null" version. - path = f"s3://{ENV.s3_staging_bucket}/{ENV.s3_filesystem_test_file_key}?versionId=null" - # The second open reads the metadata cached by the first one. - for _ in range(2): - with fs.open(path, "rb") as f: - assert f.read() == b"0123456789" + def test_read_null_version(self, fs): + path = ( + f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/" + f"filesystem/test_read_null_version/{uuid.uuid4()}" + ) + # An unversioned bucket stores each object as the "null" version, + # which an overwrite replaces. + for data in (b"1", b"22"): + fs.pipe(path, data) + for _ in range(2): + with fs.open(f"{path}?versionId=null", "rb") as f: + assert f.read() == data def test_file_url_metadata_getxattr_setxattr(self, fs): path = ( From 6e263b8c0b70d56281b1a99e6d63c28fd9e20bdf Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:12:50 +0900 Subject: [PATCH 4/6] Mention the uncached null version in the info() docstring Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index 86d839eaa..d3c256ed8 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -589,8 +589,9 @@ def info(self, path: str, **kwargs) -> S3Object: exists, with a ListObjectsV2 request (``Delimiter="/"``, ``MaxKeys=1``) that checks whether it is a key prefix; a bucket path is looked up with HeadBucket. With ``version_aware``, a cached file - entry without a version ID is looked up again. An explicit version is - cached under its version-qualified path, apart from other versions. + entry without a version ID is looked up again. An explicit version + other than ``null``, which an overwrite replaces, is cached under its + version-qualified path, apart from other versions. Args: path: S3 path (e.g., "s3://bucket" or "s3://bucket/key"). From bebe3d7c8ffcee12e6c8f859f80d90d852894dcd Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:31:36 +0900 Subject: [PATCH 5/6] Look up an explicit version only in its own HeadObject cache Cached listings and the entry of the path without a version describe the current version, so info() with a version skips them, and _head_object() keys the cache by the version-qualified path whichever way the version is given. This replaces the name comparison against the path without its version suffix. Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index d3c256ed8..4ce0d962f 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -340,6 +340,14 @@ def _head_object( ) -> S3Object | None: bucket, key, path_version_id = self.parse_path(path) version_id = path_version_id if path_version_id else version_id + if version_id and not path_version_id: + # Cache an explicit version under its version-qualified path so + # that it neither reuses nor replaces the entry of another version. + path = f"{path}?versionId={version_id}" + # Writes invalidate only the path without the version, and an + # overwrite replaces the "null" version of a bucket without + # versioning, so that version is looked up every time. + cacheable = version_id != "null" if path not in self.dircache or refresh: try: request = { @@ -366,10 +374,7 @@ def _head_object( key=key, version_id=version_id, ) - # Writes invalidate only the path without the version, and an - # overwrite replaces the "null" version of a bucket without - # versioning, so that version is looked up every time. - if path_version_id != "null": + if cacheable: self.dircache[path] = file else: file = self.dircache[path] @@ -589,9 +594,10 @@ def info(self, path: str, **kwargs) -> S3Object: exists, with a ListObjectsV2 request (``Delimiter="/"``, ``MaxKeys=1``) that checks whether it is a key prefix; a bucket path is looked up with HeadBucket. With ``version_aware``, a cached file - entry without a version ID is looked up again. An explicit version - other than ``null``, which an overwrite replaces, is cached under its - version-qualified path, apart from other versions. + entry without a version ID is looked up again. An explicit version is + looked up with HeadObject, whose result is cached under the + version-qualified path apart from other versions, except for the + ``null`` version, which an overwrite replaces. Args: path: S3 path (e.g., "s3://bucket" or "s3://bucket/key"). @@ -609,12 +615,6 @@ def info(self, path: str, **kwargs) -> S3Object: path = self._strip_protocol(path) bucket, key, path_version_id = self.parse_path(path) version_id = path_version_id if path_version_id else kwargs.pop("version_id", None) - if key and version_id and not path_version_id: - # Look up an explicit version under its version-qualified path so - # that it neither reuses nor replaces the entry of another version. - path = f"{path}?versionId={version_id}" - # Cached entries are named without the version suffix. - name = path.split("?", 1)[0] if path in ["/", ""]: return S3Object( init={ @@ -629,12 +629,14 @@ def info(self, path: str, **kwargs) -> S3Object: key=None, version_id=None, ) - if not refresh: + # Cached entries describe the current version of a path, so an + # explicit version uses only the HeadObject cache of that version. + if not refresh and not version_id: caches: list[S3Object] | S3Object | None = self._ls_from_cache(path) if caches is not None: if isinstance(caches, list): - cache = next((c for c in caches if c.name == name), None) - elif caches.name == name: + cache = next((c for c in caches if c.name == path), None) + elif caches.name == path: cache = caches else: cache = None @@ -642,7 +644,6 @@ def info(self, path: str, **kwargs) -> S3Object: if cache: if ( self.version_aware - and not version_id and cache.get("type") == S3ObjectType.S3_OBJECT_TYPE_FILE and not cache.get("version_id") ): From 46f927a4b32be5a474279f48ec765cfc57966f53 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sat, 3 Oct 2026 15:31:58 +0900 Subject: [PATCH 6/6] Describe the skipped cache for explicit versions in the info() docstring Co-Authored-By: Claude Opus 5.5 --- pyathena/filesystem/s3.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pyathena/filesystem/s3.py b/pyathena/filesystem/s3.py index 4ce0d962f..57ff4b943 100644 --- a/pyathena/filesystem/s3.py +++ b/pyathena/filesystem/s3.py @@ -594,10 +594,11 @@ def info(self, path: str, **kwargs) -> S3Object: exists, with a ListObjectsV2 request (``Delimiter="/"``, ``MaxKeys=1``) that checks whether it is a key prefix; a bucket path is looked up with HeadBucket. With ``version_aware``, a cached file - entry without a version ID is looked up again. An explicit version is - looked up with HeadObject, whose result is cached under the - version-qualified path apart from other versions, except for the - ``null`` version, which an overwrite replaces. + entry without a version ID is looked up again. With an explicit + version, the cached entries of the path are skipped, and the + HeadObject result is cached under the version-qualified path apart + from other versions, except for the ``null`` version, which an + overwrite replaces. Args: path: S3 path (e.g., "s3://bucket" or "s3://bucket/key").