From 89b5714bd894afabdf9a8363c8903a05ba84a992 Mon Sep 17 00:00:00 2001 From: Jared Yu Date: Fri, 31 Jul 2026 22:57:01 -0700 Subject: [PATCH] IO: Fix Windows drive letters misidentified as URI schemes by urlparse On Windows, Python's urlparse treats paths like 'C:\Users\...' as having scheme='c', causing 'Unrecognized filesystem type in URI: c' errors. Uses os.path.splitdrive to detect Windows drive-letter paths before urlparse is called, avoiding the misparse entirely. splitdrive is inherently platform-aware (no-op on Linux/macOS) so no sys.platform check is needed. Fixes all three parse sites (_infer_file_io_from_scheme, PyArrowFileIO.parse_location, FsspecFileIO._get_fs_from_uri) and includes platform-conditional tests. Related: #2477, #1005 --- pyiceberg/io/__init__.py | 26 ++++++++++++++++++++++---- pyiceberg/io/fsspec.py | 11 +++++++---- pyiceberg/io/pyarrow.py | 10 +++++++++- tests/io/test_io.py | 22 ++++++++++++++++++++++ tests/io/test_pyarrow.py | 12 ++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/pyiceberg/io/__init__.py b/pyiceberg/io/__init__.py index 7dbc651214..6ff1674740 100644 --- a/pyiceberg/io/__init__.py +++ b/pyiceberg/io/__init__.py @@ -27,6 +27,7 @@ import importlib import logging +import os import warnings from abc import ABC, abstractmethod from io import SEEK_SET @@ -41,6 +42,18 @@ logger = logging.getLogger(__name__) + +def _is_local_path(path: str) -> bool: + r"""Check if a path is a local filesystem path rather than a URI. + + Uses os.path.splitdrive to detect Windows drive letters (e.g. 'C:\\...') + in a platform-aware way. On non-Windows systems, splitdrive always returns + an empty drive component, so this only triggers on Windows. + """ + drive, _ = os.path.splitdrive(path) + return drive != "" + + AWS_PROFILE_NAME = "client.profile-name" AWS_REGION = "client.region" AWS_ACCESS_KEY_ID = "client.access-key-id" @@ -335,14 +348,19 @@ def _import_file_io(io_impl: str, properties: Properties) -> FileIO | None: def _infer_file_io_from_scheme(path: str, properties: Properties) -> FileIO | None: - parsed_url = urlparse(path) - if parsed_url.scheme: - if file_ios := SCHEMA_TO_FILE_IO.get(parsed_url.scheme): + if _is_local_path(path): + scheme = "file" + else: + parsed_url = urlparse(path) + scheme = parsed_url.scheme + + if scheme: + if file_ios := SCHEMA_TO_FILE_IO.get(scheme): for file_io_path in file_ios: if file_io := _import_file_io(file_io_path, properties): return file_io else: - warnings.warn(f"No preferred file implementation for scheme: {parsed_url.scheme}", stacklevel=2) + warnings.warn(f"No preferred file implementation for scheme: {scheme}", stacklevel=2) return None diff --git a/pyiceberg/io/fsspec.py b/pyiceberg/io/fsspec.py index 7749268ff5..b28ffd0699 100644 --- a/pyiceberg/io/fsspec.py +++ b/pyiceberg/io/fsspec.py @@ -89,6 +89,7 @@ InputStream, OutputFile, OutputStream, + _is_local_path, ) from pyiceberg.typedef import Properties from pyiceberg.types import strtobool @@ -443,7 +444,7 @@ def new_input(self, location: str) -> FsspecInputFile: FsspecInputFile: An FsspecInputFile instance for the given location. """ uri = urlparse(location) - fs = self._get_fs_from_uri(uri) + fs = self._get_fs_from_uri(uri, location) return FsspecInputFile(location=location, fs=fs) @override @@ -457,7 +458,7 @@ def new_output(self, location: str) -> FsspecOutputFile: FsspecOutputFile: An FsspecOutputFile instance for the given location. """ uri = urlparse(location) - fs = self._get_fs_from_uri(uri) + fs = self._get_fs_from_uri(uri, location) return FsspecOutputFile(location=location, fs=fs) @override @@ -475,11 +476,13 @@ def delete(self, location: str | InputFile | OutputFile) -> None: str_location = location uri = urlparse(str_location) - fs = self._get_fs_from_uri(uri) + fs = self._get_fs_from_uri(uri, str_location) fs.rm(str_location) - def _get_fs_from_uri(self, uri: "ParseResult") -> AbstractFileSystem: + def _get_fs_from_uri(self, uri: "ParseResult", location: str = "") -> AbstractFileSystem: """Get a filesystem from a parsed URI, using hostname for ADLS account resolution.""" + if _is_local_path(location): + return self.get_fs("file") if uri.scheme in _ADLS_SCHEMES: return self.get_fs(uri.scheme, uri.hostname) return self.get_fs(uri.scheme) diff --git a/pyiceberg/io/pyarrow.py b/pyiceberg/io/pyarrow.py index 29ede3ce9e..c36f1639d9 100644 --- a/pyiceberg/io/pyarrow.py +++ b/pyiceberg/io/pyarrow.py @@ -121,6 +121,7 @@ InputStream, OutputFile, OutputStream, + _is_local_path, ) from pyiceberg.io.fileformat import DataFileStatistics as DataFileStatistics from pyiceberg.io.fileformat import FileFormatFactory, FileFormatModel, FileFormatWriter @@ -401,10 +402,17 @@ def __init__(self, properties: Properties = EMPTY_DICT): @staticmethod def parse_location(location: str, properties: Properties = EMPTY_DICT) -> tuple[str, str, str]: - """Return (scheme, netloc, path) for the given location. + r"""Return (scheme, netloc, path) for the given location. Uses DEFAULT_SCHEME and DEFAULT_NETLOC if scheme/netloc are missing. + On Windows, paths with drive letters (e.g. 'C:\\...') are treated as + local file paths rather than URIs. """ + if _is_local_path(location): + default_scheme = properties.get("DEFAULT_SCHEME", "file") + default_netloc = properties.get("DEFAULT_NETLOC", "") + return default_scheme, default_netloc, os.path.abspath(location) + uri = urlparse(location) if not uri.scheme: diff --git a/tests/io/test_io.py b/tests/io/test_io.py index d9bee33f8b..a3dd9765c8 100644 --- a/tests/io/test_io.py +++ b/tests/io/test_io.py @@ -17,6 +17,7 @@ import os import pickle +import sys import tempfile from typing import Any @@ -27,6 +28,7 @@ PY_IO_IMPL, _import_file_io, _infer_file_io_from_scheme, + _is_local_path, load_file_io, ) from pyiceberg.io.pyarrow import PyArrowFileIO @@ -339,3 +341,23 @@ def test_infer_file_io_from_schema_unknown() -> None: _infer_file_io_from_scheme("unknown://bucket/path/", {}) assert str(w[0].message) == "No preferred file implementation for scheme: unknown" + + +@pytest.mark.parametrize( + "path", + ["s3://bucket/key", "hdfs://cluster/path", "gs://bucket/obj", "/tmp/foo", ""], +) +def test_is_local_path_false_for_uris_and_posix(path: str) -> None: + assert _is_local_path(path) is False + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only behavior") +@pytest.mark.parametrize("path", [r"C:\Users\test", r"D:\data\iceberg", "E:/warehouse"]) +def test_is_local_path_true_on_windows(path: str) -> None: + assert _is_local_path(path) is True + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only behavior") +def test_infer_file_io_from_scheme_windows_path() -> None: + result = _infer_file_io_from_scheme(r"C:\Users\test\warehouse", {}) + assert isinstance(result, PyArrowFileIO) diff --git a/tests/io/test_pyarrow.py b/tests/io/test_pyarrow.py index fef33a25c7..407f925ed2 100644 --- a/tests/io/test_pyarrow.py +++ b/tests/io/test_pyarrow.py @@ -17,6 +17,7 @@ # pylint: disable=protected-access,unused-argument,redefined-outer-name import logging import os +import sys import tempfile import uuid import warnings @@ -2326,6 +2327,17 @@ def check_results(location: str, expected_schema: str, expected_netloc: str, exp check_results("/root/tmp/foo.txt", "file", "", "/root/tmp/foo.txt") +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only behavior") +def test_parse_location_windows_drive_letter() -> None: + """Windows drive letters should be treated as local file paths, not URL schemes.""" + for drive in ("C", "D", "c", "d"): + path = f"{drive}:\\Users\\test\\file.avro" + scheme, netloc, result_path = PyArrowFileIO.parse_location(path) + assert scheme == "file" + assert netloc == "" + assert result_path == os.path.abspath(path) + + def test_make_compatible_name() -> None: assert make_compatible_name("label/abc") == "label_x2Fabc" assert make_compatible_name("label?abc") == "label_x3Fabc"