From a7bbda7b8f8d1b06821aa77915fd5cf12f39d75d Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Sat, 4 Jul 2026 21:47:20 +0530 Subject: [PATCH] fix: build StaticTable version-hint metadata locations with forward slashes StaticTable._metadata_location_from_version_hint built the version-hint and resolved metadata locations with os.path.join. Iceberg locations are URIs and must always be forward-slash separated, but os.path.join uses the OS separator, so on Windows it produced backslash-separated keys such as s3://.../taxis\metadata\version-hint.text. That is an invalid object-store key, which breaks the documented "pass the table root path" usage of StaticTable.from_metadata on Windows. Join the components with explicit forward slashes, matching how locations are built elsewhere in pyiceberg (for example pyiceberg/table/locations.py). Behavior is unchanged on POSIX. The now-unused os import is removed. Add an OS-independent regression test that swaps os.path.join for the Windows join so the metadata location is asserted to stay a valid forward-slash URI on any platform (the existing test only exercised local temp paths, which use the POSIX separator on Linux CI and never caught this). Related: #2477 --- pyiceberg/table/__init__.py | 10 ++++---- tests/table/test_init.py | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/pyiceberg/table/__init__.py b/pyiceberg/table/__init__.py index 63b87d290e..d1e2088583 100644 --- a/pyiceberg/table/__init__.py +++ b/pyiceberg/table/__init__.py @@ -17,7 +17,6 @@ from __future__ import annotations import itertools -import os import uuid import warnings from abc import ABC, abstractmethod @@ -1788,7 +1787,8 @@ def refresh(self) -> Table: @classmethod def _metadata_location_from_version_hint(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> str: - version_hint_location = os.path.join(metadata_location, "metadata", "version-hint.text") + metadata_dir = f"{metadata_location.rstrip('/')}/metadata" + version_hint_location = f"{metadata_dir}/version-hint.text" io = load_file_io(properties=properties, location=version_hint_location) file = io.new_input(version_hint_location) @@ -1796,11 +1796,11 @@ def _metadata_location_from_version_hint(cls, metadata_location: str, properties content = stream.read().decode("utf-8") if content.endswith(".metadata.json"): - return os.path.join(metadata_location, "metadata", content) + return f"{metadata_dir}/{content}" elif content.isnumeric(): - return os.path.join(metadata_location, "metadata", f"v{content}.metadata.json") + return f"{metadata_dir}/v{content}.metadata.json" else: - return os.path.join(metadata_location, "metadata", f"{content}.metadata.json") + return f"{metadata_dir}/{content}.metadata.json" @classmethod def from_metadata(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> StaticTable: diff --git a/tests/table/test_init.py b/tests/table/test_init.py index 1670e62587..a6e6c14d2d 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -16,6 +16,8 @@ # under the License. # pylint:disable=redefined-outer-name import json +import ntpath +import os import uuid from copy import copy from typing import Any @@ -589,6 +591,53 @@ def test_static_table_version_hint_same_as_table( assert static_table.metadata == table_v2.metadata +@pytest.mark.parametrize( + "version_hint_content, expected_metadata_file", + [ + ("v3.metadata.json", "v3.metadata.json"), + ("3", "v3.metadata.json"), + ("some-uuid", "some-uuid.metadata.json"), + ], +) +def test_static_table_version_hint_location_uses_forward_slashes( + version_hint_content: str, expected_metadata_file: str, monkeypatch: pytest.MonkeyPatch +) -> None: + # Iceberg locations are URIs and must always be forward-slash separated, regardless of the + # host OS. Swapping in the Windows path join makes an OS-separator join emit backslashes, so + # this asserts the resolved metadata location stays a valid forward-slash URI on any platform. + monkeypatch.setattr(os.path, "join", ntpath.join) + + table_root = "s3://warehouse/wh/nyc.db/taxis" + requested_locations: list[str] = [] + + class _FakeStream: + def __enter__(self) -> "_FakeStream": + return self + + def __exit__(self, *args: Any) -> None: + return None + + def read(self, size: int = 0) -> bytes: + return version_hint_content.encode("utf-8") + + class _FakeInputFile: + def open(self, seekable: bool = True) -> "_FakeStream": + return _FakeStream() + + class _FakeFileIO: + def new_input(self, location: str) -> "_FakeInputFile": + requested_locations.append(location) + return _FakeInputFile() + + monkeypatch.setattr("pyiceberg.table.load_file_io", lambda *args, **kwargs: _FakeFileIO()) + + resolved_location = StaticTable._metadata_location_from_version_hint(table_root) + + assert requested_locations == [f"{table_root}/metadata/version-hint.text"] + assert resolved_location == f"{table_root}/metadata/{expected_metadata_file}" + assert "\\" not in resolved_location + + def test_static_table_io_does_not_exist(metadata_location: str) -> None: with pytest.raises(ValueError): StaticTable.from_metadata(metadata_location, {PY_IO_IMPL: "pyiceberg.does.not.exist.FileIO"})