Skip to content

Commit c3a97f6

Browse files
committed
Fix file:// URL handling in _register_object_store_for_path
- Allow empty netloc for file:// scheme (e.g. file:///tmp/data.parquet) - Require netloc (host/bucket) only for non-file schemes (s3, gs, az, https) - Add doctest +SKIP to register_parquet example referencing nonexistent file - Use Path.as_uri() in end-to-end tests for portable file:// URL construction - Add tests for file:// acceptance and scheme-without-host rejection
1 parent beaa3df commit c3a97f6

2 files changed

Lines changed: 34 additions & 8 deletions

File tree

python/datafusion/context.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -624,21 +624,32 @@ def _register_object_store_for_path(
624624
parameter is provided.
625625
626626
Args:
627-
path: A URL-style path (e.g. ``"s3://bucket/key.parquet"``).
627+
path: A URL-style path (e.g. ``"s3://bucket/key.parquet"`` or
628+
``"file:///tmp/data.parquet"``).
628629
store: An object store instance to register.
629630
630631
Raises:
631-
ValueError: If the path does not contain a recognized URL scheme.
632+
ValueError: If the path does not contain a URL scheme, or if
633+
a non-file scheme is missing a host/bucket component.
632634
"""
633635
parsed = urlparse(str(path))
634-
if not parsed.scheme or not parsed.netloc:
636+
if not parsed.scheme:
635637
msg = (
636638
f"Cannot determine object store URL from path {path!r}. "
637639
"The path must use a URL scheme (e.g. 's3://bucket/key')."
638640
)
639641
raise ValueError(msg)
642+
# file:// URLs typically have an empty netloc (e.g. file:///tmp/a.parquet)
643+
# For other schemes (s3, gs, az, https) the netloc (bucket/host) is required.
644+
if parsed.scheme != "file" and not parsed.netloc:
645+
msg = (
646+
f"Cannot determine object store URL from path {path!r}. "
647+
"The path must include a host or bucket "
648+
"(e.g. 's3://bucket/key')."
649+
)
650+
raise ValueError(msg)
640651
scheme = f"{parsed.scheme}://"
641-
host = parsed.netloc
652+
host = parsed.netloc or None
642653
self.register_object_store(scheme, store, host=host)
643654

644655
def register_listing_table(
@@ -1096,7 +1107,7 @@ def register_parquet(
10961107
10971108
>>> import datafusion
10981109
>>> ctx = datafusion.SessionContext()
1099-
>>> ctx.register_parquet("my_table", "data.parquet")
1110+
>>> ctx.register_parquet("my_table", "data.parquet") # doctest: +SKIP
11001111
11011112
Register from S3 with inline credentials (thread-safe):
11021113

python/tests/test_object_store_param.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,26 @@ def test_raises_on_relative_path(self, ctx):
8484

8585
def test_raises_on_windows_path(self, ctx):
8686
mock_store = MagicMock()
87-
with pytest.raises(ValueError, match="Cannot determine object store URL"):
87+
with pytest.raises(ValueError, match="must include a host or bucket"):
8888
ctx._register_object_store_for_path(
8989
"C:\\Users\\data\\file.parquet", mock_store
9090
)
9191

92+
def test_raises_on_scheme_without_host_for_non_file(self, ctx):
93+
"""Non-file schemes (s3, gs, etc.) require a host/bucket."""
94+
mock_store = MagicMock()
95+
with pytest.raises(ValueError, match="must include a host or bucket"):
96+
ctx._register_object_store_for_path("s3:///key.parquet", mock_store)
97+
98+
def test_parses_file_url(self, ctx):
99+
"""file:// URLs with empty netloc should be accepted."""
100+
mock_store = MagicMock()
101+
with patch.object(ctx, "register_object_store") as mock_register:
102+
ctx._register_object_store_for_path(
103+
"file:///tmp/path/to/file.parquet", mock_store
104+
)
105+
mock_register.assert_called_once_with("file://", mock_store, host=None)
106+
92107
def test_accepts_pathlib_path_raises(self, ctx):
93108
"""pathlib.Path cannot represent URLs, so this should raise."""
94109
mock_store = MagicMock()
@@ -258,7 +273,7 @@ def test_register_parquet_with_local_object_store(self, ctx, tmp_path):
258273

259274
# Use file:// URL with LocalFileSystem object store
260275
store = LocalFileSystem()
261-
file_url = f"file://{tmp_path}/test.parquet"
276+
file_url = parquet_path.as_uri()
262277

263278
ctx.register_parquet("test_tbl", file_url, object_store=store)
264279
result = ctx.sql("SELECT * FROM test_tbl").collect()
@@ -277,7 +292,7 @@ def test_read_parquet_with_local_object_store(self, ctx, tmp_path):
277292
pq.write_table(table, str(parquet_path))
278293

279294
store = LocalFileSystem()
280-
file_url = f"file://{tmp_path}/read_test.parquet"
295+
file_url = parquet_path.as_uri()
281296

282297
df = ctx.read_parquet(file_url, object_store=store)
283298
result = df.collect()

0 commit comments

Comments
 (0)