aglinxinyuan opened a new pull request, #6545:
URL: https://github.com/apache/texera/pull/6545
### What changes were proposed in this PR?
Follow-up to #6488, which made Iceberg local storage work on Windows without
winutils **on the Scala/JVM side only** (`IcebergUtil` +
`WinutilsFreeLocalFileSystem`).
The Python UDF worker writes its output-port Iceberg storage through its
**own**
pyiceberg path — `core/storage/iceberg/iceberg_document.py` and
`document_factory.py`
call `IcebergCatalogInstance.get_instance()` → `create_postgres_catalog`
inside the
Python process — so #6488 does not reach it. On a Windows dev machine with a
local-filesystem warehouse (`postgres` catalog type), that path fails two
ways:
| # | Symptom | Root cause |
|---|---|---|
| 1 | `ValueError: Unrecognized filesystem type in URI: c` at table creation
| a bare drive path `C:\...` is parsed by pyiceberg as URI scheme `c` |
| 2 | `OSError: [WinError 123] ... The filename, directory name, or volume
label syntax is incorrect` | even a `file:///C:/...` URI is rejected by
pyiceberg's default **pyarrow** FileIO (`/C:/...`) |
Fix (in `iceberg_utils.py`, mirroring #6488's tight, self-gating approach):
| Change | Detail |
|---|---|
| `_is_windows_local_warehouse(warehouse)` (new) | True only when the
warehouse carries a **Windows drive letter** — bare (`C:\...`, `C:/...`) or
`file:///C:/...`. Excludes POSIX paths, colon-stripped paths (`C/...`), and
remote object stores (`s3://...`). |
| `_to_file_uri(warehouse)` (new) | Normalizes a bare drive path to a
`file:///` URI via `PureWindowsPath.as_uri()` (deterministic on every host OS);
already-URI values pass through. |
| `create_postgres_catalog` | When (and only when) the warehouse is
Windows-local, register the normalized `file:///` URI and select
`pyiceberg.io.fsspec.FsspecFileIO`, whose `LocalFileSystem` handles Windows
drive paths. |
| `create_rest_catalog` | Unchanged — its `warehouse_name` is a logical
identifier resolved server-side via `S3FileIO`, not a local path (checked,
exempt). |
The gate is deliberately narrow so the fix cannot alter the cross-runtime
warehouse convention that Linux, macOS, CI, and the Scala engine rely on:
PyIceberg
persists the `warehouse` value into table metadata, so a `postgres` catalog
with a
POSIX or remote warehouse must keep the plain-path / default-FileIO behavior
(regression fixed in #4409). Only genuine Windows drive-letter warehouses are
normalized.
The drive path is normalized with `PureWindowsPath.as_posix()` (not
`.as_uri()`)
so a warehouse containing a space — e.g. `C:\Users\John Doe\...`, `C:\Program
Files\...`, OneDrive-redirected profiles — keeps a **raw** space rather than
`%20`; fsspec's `LocalFileSystem` does not URL-decode, so a `%20` would write
into a literally `%20`-named directory.
Scope note: on Windows the two runtimes register different warehouse strings
into
the shared `postgres` catalog — the Scala side its existing colon-stripped
path
(`C/Users/...`), the Python worker the absolute `file:///C:/...` URI. This
is fine
for the actual data flow here (a Python UDF's output-port table is written
by the
Python worker and read back by the engine/result service via the absolute
metadata
pointer, verified end-to-end); reconciling the Scala side's colon-stripped
convention is out of scope for this fix and would belong with #6488's
follow-ups.
Before → after:
| Environment (`postgres` catalog) | Before | After |
|---|---|---|
| Windows, local warehouse (`C:\...`) | worker crashes at first table create
| works |
| Linux / macOS / CI, local warehouse (`/tmp/...`) | works | unchanged (gate
off) |
| Any host, remote warehouse (`s3://...`) | works | unchanged (gate off) |
| `rest` catalog | unaffected (`S3FileIO`) | unaffected |
### Any related issues, documentation, discussions?
Follow-up to #6488 (Scala/JVM side); closes the Python-worker half of #6487.
Preserves the cross-runtime warehouse invariant from #4409.
### How was this PR tested?
- New `TestCreatePostgresCatalogWindowsLocal` in
`test_iceberg_utils_catalog.py`
(written first, TDD): a Windows drive-letter warehouse is normalized to a
`file:///` URI and `FsspecFileIO` is selected; POSIX, colon-stripped, and
remote (`s3://`) warehouses are left untouched (no FileIO override). The
tests
assert on the computed `SqlCatalog` props and use `PureWindowsPath`, so
they are
OS-independent and pass on Linux CI.
- The pre-existing `TestCreatePostgresCatalog` tests (the #4409 plain-path
invariant) still pass unchanged.
- Verified end-to-end on a real Windows filesystem: a bare `C:\...` warehouse
fed through the productionized helpers into a real catalog creates a table
and
round-trips rows (both failure modes above reproduced on stock `main`
first).
- `amber/` `ruff check` and `ruff format --check` pass on the changed files.
(The `test_iceberg_document.py` / REST integration tests require a live
postgres / REST catalog server and are environment-gated locally; they are
unaffected by this change — confirmed identical pass/fail with the diff
stashed.)
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
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]
