Skip to content

Conversation

nikhilsinhaparseable
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Oct 16, 2025

When DataFusion tries to query a date range where no manifest exists,
it calls into the listing table functionality
The listing table builder tries to generate prefixes and list directories for those date ranges

Previously, if those directories didn't exist,
it would return an error that propagated up as a DataFusion External error

Execution Error: Query Execution failed due to error in datafusion: External error: IO Error: No such file or directory (os error 2)

Now, it gracefully returns an empty list,
allowing the query to continue and return no results instead of failing

Summary by CodeRabbit

  • Bug Fixes
    • Improved resilience when accessing missing directories. File listing operations now gracefully return empty results instead of failing when directories don't exist.

When DataFusion tries to query a date range where no manifest exists,
it calls into the listing table functionality
The listing table builder tries to generate prefixes and list directories for those date ranges

Previously, if those directories didn't exist,
it would return an error that propagated up as a DataFusion External error
```
Execution Error: Query Execution failed due to error in datafusion: External error: IO Error: No such file or directory (os error 2)
```

Now, it gracefully returns an empty list,
allowing the query to continue and return no results instead of failing
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

Modified error handling in three directory listing methods within the local filesystem storage module to return empty vectors instead of errors when directories don't exist, treating missing directories as valid empty-result scenarios rather than error conditions.

Changes

Cohort / File(s) Summary
NotFound Error Handling
src/storage/localfs.rs
Added special-case error handling in list_dirs, list_dirs_relative, and list_dates methods to return Ok(Vec::new()) when fs::read_dir operations encounter NotFound errors, replacing error propagation with empty result behavior

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The changes follow a consistent, repetitive pattern applied across three related methods with straightforward error-handling logic and no impact to public API contracts.

Poem

🐰 A rabbit hops through missing paths with grace,
No error screams from an absent place,
Just empty paws, an empty list returned,
A gentle touch when directories burned. 🌙

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not adhere to the repository’s template because it omits the required “Fixes #XXXX” issue reference, lacks the “### Description” heading with the PR goal, rationale, and key changes, and does not include the checklist for testing, comments, and documentation. Please revise the description to add a “Fixes #XXXX” line if there is an associated issue, include the “### Description” section detailing the PR’s objective, chosen solution, and key changes, and incorporate the checklist of test, comment, and documentation items from the repository template.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly identifies the main change by indicating a fix for a directory read error in the localfs implementation, which aligns directly with the pull request’s adjustment to handle NotFound errors when listing directories.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8bf618 and 08577ea.

📒 Files selected for processing (1)
  • src/storage/localfs.rs (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Build Default x86_64-apple-darwin
  • GitHub Check: Build Default x86_64-pc-windows-msvc
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
  • GitHub Check: Build Kafka aarch64-apple-darwin
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • GitHub Check: Build Default aarch64-apple-darwin
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu
  • GitHub Check: coverage
🔇 Additional comments (3)
src/storage/localfs.rs (3)

474-477: LGTM! Graceful handling of missing directories.

The NotFound error handling correctly returns an empty vector when the root directory doesn't exist, allowing DataFusion queries to continue instead of failing. This aligns with the PR objective.


508-511: LGTM! Consistent error handling pattern.

The NotFound handling is correctly applied, consistent with the changes in list_dirs. This ensures uniform behavior across directory listing methods.


542-545: LGTM! Fixes the reported DataFusion error.

This change directly addresses the issue described in the PR where DataFusion queries failed when attempting to list non-existent date directories. The empty vector return allows queries to complete successfully with no results.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nitisht nitisht merged commit 3dd82e4 into parseablehq:main Oct 16, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants