duckdb / duckdb/duckdb-python

Translate unsupported fsspec modification times into a typed DuckDB error

Aperta
#584 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
187
Fork
112
Merge medio
13h 29m
PR unite (30g)
17

Descrizione

## 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.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia da PythonFilesystem::GetLastModifiedTime() e traccia il modo in cui DirectFileReader gestisce gli errori dei metadati opzionali. Aggiungi un test di regressione usando un filesystem fsspec personalizzato con un comportamento modified() non supportato, quindi eseguilo tramite read_blob(...).last_modified. Il lavoro è completato quando gli orari di modifica non supportati restituiscono NULL, mentre gli errori del filesystem non correlati continuano a propagarsi.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
cpp, python
Ambito
database, testing
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Attiva
Chiarezza
Abbastanza chiara
Idoneità per principianti
68/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.