apache / apache/datafusion

Add Result return types to CatalogProvider / SchemaProvider fallible methods

Open
#21,778 2 comments 0 reactions 1 assignee Claimed by @gratus00 View on GitHub
enhancement
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Is your feature request related to a problem or challenge?

Several methods on `CatalogProvider` and `SchemaProvider` perform potentially fallible operations but have no way to report errors:

| Trait | Method | Current signature |
|-------|--------|-------------------|
| `CatalogProvider` | `schema_names()` | `-> Vec` |
| `CatalogProvider` | `schema()` | `-> Option>` |
| `SchemaProvider` | `table_names()` | `-> Vec` |
| `SchemaProvider` | `table_exist()` | `-> bool` |

For in-memory catalogs this is fine, but remote catalog implementations (e.g. [paimon-rust](https://github.com/apache/paimon-rust), delta-rs Unity Catalog) need to make network
calls in these methods. When a request fails (403, timeout, connection refused, etc.), the implementation has no choice but to silently return an empty vec / `None` / `false`,
which is indistinguishable from "no schemas exist" or "table not found".

This makes debugging very difficult — a network blip looks identical to an empty catalog.

### Describe the solution you'd like

Change the return types to `Result<...>`:

```rust
// CatalogProvider
fn schema_names(&self) -> Result>;
fn schema(&self, name: &str) -> Result>>;

// SchemaProvider
fn table_names(&self) -> Result>;
fn table_exist(&self, name: &str) -> Result;
```
This is a breaking API change. All existing implementations would need to wrap their return values in Ok(...). The migration is mechanical but touches every catalog implementation.

### Describe alternatives you've considered

1. Logging (current workaround): Add log::error! before returning the fallback value. This surfaces the error for debugging but callers still can't react to it.
2. async versions of these methods: Would also solve the problem but is a much larger change and has been intentionally avoided for planning performance reasons (as documented in CatalogProvider's doc comments).
3. Keep as-is: Accept that remote catalogs must silently swallow errors in these methods.

### Additional context
The other methods on these traits (table(), register_schema(), deregister_schema(), register_table(), deregister_table()) already return Result, so this would make the API consistent.

Affected files (estimated): ~21 source files + FFI boundary in datafusion-ffi.

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.