aglinxinyuan commented on code in PR #6545:
URL: https://github.com/apache/texera/pull/6545#discussion_r3609887696
##########
amber/src/main/python/core/storage/iceberg/iceberg_utils.py:
##########
@@ -35,6 +39,58 @@
# Suffix used to encode LARGE_BINARY fields in Iceberg (must match Scala
IcebergUtil)
LARGE_BINARY_FIELD_SUFFIX = "__texera_large_binary_ptr"
+# pyiceberg FileIO whose fsspec LocalFileSystem handles Windows drive paths,
+# which the default pyarrow FileIO cannot. See `_is_windows_local_warehouse`.
+_FSSPEC_FILE_IO = "pyiceberg.io.fsspec.FsspecFileIO"
+
+# Matches a filesystem path that starts with a Windows drive letter and a
+# separator, e.g. `C:\...` or `C:/...` (but not the drive-relative `C:foo`).
+_WINDOWS_DRIVE_PATH = re.compile(r"^[A-Za-z]:[\\/]")
+
+
+def _is_windows_local_warehouse(warehouse: str) -> bool:
+ """
+ Whether ``warehouse`` is a LOCAL filesystem path carrying a Windows drive
+ letter, either as a bare path (``C:\\...`` / ``C:/...``) or a ``file://``
+ URI (``file:///C:/...``).
+
+ Remote object stores (``s3://...``), POSIX paths, drive-less relative
paths,
+ and colon-stripped paths (the form the Scala side registers, ``C/...``) are
+ excluded, so the default pyarrow FileIO / plain-path convention that Linux,
+ macOS, CI, and the Scala engine rely on is left untouched (see #4409).
+ """
+ if not warehouse:
+ return False
+ text = str(warehouse)
+ if _WINDOWS_DRIVE_PATH.match(text):
+ return True
+ parsed = urlparse(text)
+ if parsed.scheme.lower() == "file": # `file` URI schemes are
case-insensitive
+ return bool(_WINDOWS_DRIVE_PATH.match(parsed.path.lstrip("/")))
+ return False
+
+
+def _to_file_uri(warehouse: str) -> str:
+ """
+ Normalize a bare Windows drive path (``C:\\...``) to a ``file:///`` URI so
+ pyiceberg does not mis-parse the drive letter as a URI scheme (``c://`` ->
+ "Unrecognized filesystem type in URI: c"). Values that are already URIs
+ (e.g. ``file:///C:/...``) are returned unchanged.
+
+ The URI is built from ``PureWindowsPath.as_posix()`` rather than
+ ``.as_uri()`` so the path is *not* percent-encoded: the fsspec
+ ``LocalFileSystem`` behind ``FsspecFileIO`` does not URL-decode, so an
+ encoded space (``C:\\Users\\John Doe`` -> ``.../John%20Doe``) would send
+ writes to a literally ``%20``-named directory. ``PureWindowsPath`` keeps
the
+ conversion deterministic on every host OS (the result is identical off
+ Windows).
+ """
+ text = str(warehouse)
+ if _WINDOWS_DRIVE_PATH.match(text):
+ return "file:///" + PureWindowsPath(text).as_posix()
Review Comment:
Good catch — refreshed the description so the fix table reads
`PureWindowsPath.as_posix()`, matching the code and the paragraph below it.
Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
