Skip to content

Commit 4a4baea

Browse files
committed
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
1 parent 1312c55 commit 4a4baea

2 files changed

Lines changed: 48 additions & 5 deletions

File tree

‎pyiceberg/table/__init__.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import itertools
2020
import logging
21-
import os
2221
import random
2322
import time
2423
import uuid
@@ -1963,19 +1962,20 @@ def refresh(self) -> Table:
19631962

19641963
@classmethod
19651964
def _metadata_location_from_version_hint(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> str:
1966-
version_hint_location = os.path.join(metadata_location, "metadata", "version-hint.text")
1965+
metadata_dir = f"{metadata_location.rstrip('/')}/metadata"
1966+
version_hint_location = f"{metadata_dir}/version-hint.text"
19671967
io = load_file_io(properties=properties, location=version_hint_location)
19681968
file = io.new_input(version_hint_location)
19691969

19701970
with file.open() as stream:
19711971
content = stream.read().decode("utf-8")
19721972

19731973
if content.endswith(".metadata.json"):
1974-
return os.path.join(metadata_location, "metadata", content)
1974+
return f"{metadata_dir}/{content}"
19751975
elif content.isnumeric():
1976-
return os.path.join(metadata_location, "metadata", f"v{content}.metadata.json")
1976+
return f"{metadata_dir}/v{content}.metadata.json"
19771977
else:
1978-
return os.path.join(metadata_location, "metadata", f"{content}.metadata.json")
1978+
return f"{metadata_dir}/{content}.metadata.json"
19791979

19801980
@classmethod
19811981
def from_metadata(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> StaticTable:

‎tests/table/test_init.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,49 @@ def test_static_table_version_hint_same_as_table(
590590
assert static_table.metadata == table_v2.metadata
591591

592592

593+
@pytest.mark.parametrize(
594+
"version_hint_content, expected_metadata_file",
595+
[
596+
("v3.metadata.json", "v3.metadata.json"),
597+
("3", "v3.metadata.json"),
598+
("some-uuid", "some-uuid.metadata.json"),
599+
],
600+
)
601+
def test_static_table_version_hint_location_uses_forward_slashes(
602+
version_hint_content: str, expected_metadata_file: str, monkeypatch: pytest.MonkeyPatch
603+
) -> None:
604+
# Iceberg locations are URIs and must always be forward-slash separated, regardless of the host OS.
605+
table_root = "s3://warehouse/wh/nyc.db/taxis"
606+
requested_locations: list[str] = []
607+
608+
class _FakeStream:
609+
def __enter__(self) -> "_FakeStream":
610+
return self
611+
612+
def __exit__(self, *args: Any) -> None:
613+
return None
614+
615+
def read(self, size: int = 0) -> bytes:
616+
return version_hint_content.encode("utf-8")
617+
618+
class _FakeInputFile:
619+
def open(self, seekable: bool = True) -> "_FakeStream":
620+
return _FakeStream()
621+
622+
class _FakeFileIO:
623+
def new_input(self, location: str) -> "_FakeInputFile":
624+
requested_locations.append(location)
625+
return _FakeInputFile()
626+
627+
monkeypatch.setattr("pyiceberg.table.load_file_io", lambda *args, **kwargs: _FakeFileIO())
628+
629+
resolved_location = StaticTable._metadata_location_from_version_hint(table_root)
630+
631+
assert requested_locations == [f"{table_root}/metadata/version-hint.text"]
632+
assert resolved_location == f"{table_root}/metadata/{expected_metadata_file}"
633+
assert "\\" not in resolved_location
634+
635+
593636
def test_static_table_io_does_not_exist(metadata_location: str) -> None:
594637
with pytest.raises(ValueError):
595638
StaticTable.from_metadata(metadata_location, {PY_IO_IMPL: "pyiceberg.does.not.exist.FileIO"})

0 commit comments

Comments
 (0)