duckdb / duckdb/duckdb-python

Translate unsupported fsspec modification times into a typed DuckDB error

Open
#584 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
187
Forks
112
Avg merge
13h 29m
Merged PRs (30d)
17

Description

## What happens?

When a filesystem registered through `Connection.register_filesystem()` cannot provide a modification time, querying the `last_modified` column of `read_blob` surfaces the raw Python exception instead of returning `NULL`.

One concrete case is `gcsfs`: `GCSFileSystem.modified()` indexes `info(path)["mtime"]`, while some GCS object metadata (and synthesized directory entries) has no `mtime`. The resulting `KeyError: 'mtime'` currently reaches a DuckDB query as an untyped error and can abort callers such as DuckLake `CHECKPOINT`.

This is the Python/fsspec half of:

- duckdb/ducklake#1042
- duckdb/ducklake#1336

## Why the error loses its type

`PythonFilesystem::GetLastModifiedTime()` currently calls `filesystem.modified()` directly. A Python exception crosses the nanobind boundary as `std::exception`; when DuckDB later constructs `ErrorData(ex)`, it has `ExceptionType::INVALID`, so extensions cannot distinguish unsupported modification-time metadata from unrelated I/O failures without inspecting the message.

DuckDB core already has the desired downstream behavior: `DirectFileReader` converts `ExceptionType::NOT_IMPLEMENTED` failures for optional file metadata columns to `NULL`. An `older_than` predicate then naturally retains only files whose age is unknown.

## Suggested direction

Translate the Python filesystem capability error at the adapter boundary in `PythonFilesystem::GetLastModifiedTime()`:

1. Catch `nb::python_error`.
2. Map Python `NotImplementedError` to DuckDB `NotImplementedException`.
3. Consider a narrowly scoped compatibility mapping for the current gcsfs `KeyError('mtime')`, or coordinate with gcsfs so missing `mtime` is reported as `NotImplementedError`.
4. Re-throw every unrelated Python exception unchanged.

Please add a regression test with a small custom fsspec filesystem whose `modified()` is unsupported, asserting that `read_blob(...).last_modified` is `NULL` while other filesystem errors still surface.

This keeps provider-specific exception classification in the Python filesystem adapter instead of requiring downstream extensions to accumulate message-matching helpers.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.