TempTableGuard is armed after the load its doc comment claims to protect
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 48/100
Research direction
Start in hyperdb-mcp/src/ingest.rs by reading TempTableGuard, replace_load, and merge_via_temp_table around the cited lines. Trace the guard's borrow lifetime and the load-to-cleanup boundary, then review the existing cleanup behavior and comments. Done means the documented panic coverage matches the agreed implementation, with no misleading guarantee remaining.
Written by the indexing model from the issue text.
Description
Summary
TempTableGuard's doc comment says it drops the merge temp table even if a per-format ingest "panics mid-load," but TempTableGuard::new is called after the load returns — so a panic inside the load is not covered.
The claim, twice:
// hyperdb-mcp/src/ingest.rs:162-168
/// RAII guard that ensures a temp table is dropped on **every** scope
/// exit — `Ok`, `Err`, *and* panic-unwind. Used by
/// [`merge_via_temp_table`] so an orphan `__hyperdb_merge_*` table
/// can't leak into the workspace, even if a per-format ingest path
/// panics mid-load.
// hyperdb-mcp/src/ingest.rs:260-262
/// 2. Load incoming data into a unique temp table via `replace_load`.
/// A `TempTableGuard` arms here and unwind-safely drops the temp
/// on every exit (`Ok` / `Err` / panic).
The construction site:
// hyperdb-mcp/src/ingest.rs:377-383
let tmp_result = replace_load(engine, &tmp_opts)?;
// Arm the cleanup guard immediately after the load so any later
// failure (or panic) drops the temp table on unwind. The guard
// tracks `target_db` so the DROP lands in the same DB the temp
// was created in.
let mut guard = TempTableGuard::new(engine, tmp.clone(), opts.target_db.clone());
The inline comment at :379-380 is accurate about the timing — "immediately after the load … so any later failure." The two doc comments above it are not. Everything from :383 onward genuinely is covered: the table_exists_in contract check, the column-metadata reads, the ALTER TABLE ADD COLUMN pass, and the DELETE/INSERT pair. The uncovered window is replace_load itself, which is exactly the window the doc names.
Impact
Latent. A replace_load that panics after creating the temp table but before returning leaves an orphan __hyperdb_merge_<target>_<pid>_<nanos>_<counter> table in the target database, with no cleanup and no log line — the guard's panic-path reporting at hyperdb-mcp/src/ingest.rs:216-227 was never constructed. Temp names are unique per call, so these accumulate rather than colliding: one per panicking merge, until someone notices them in a table listing. No data corruption and no wrong answers. The real cost is a doc comment that will convince the next reader the window is already closed.
Fix direction
This is not a one-line move. The guard borrows the engine immutably for its lifetime:
// hyperdb-mcp/src/ingest.rs:172-173
struct TempTableGuard<'a> {
engine: &'a Engine,
while replace_load needs an exclusive borrow (F: FnOnce(&mut Engine, &IngestOptions), hyperdb-mcp/src/ingest.rs:304). Constructing the guard first would hold a shared borrow across the closure's exclusive one, which the borrow checker rejects — presumably why it sits where it does.
So the choice is either to restructure so the guard can span the load — have it hold something cheaply shareable instead of an &Engine, or register the temp name in a scope-owned list that a single outer guard drains — or to correct the two doc comments to promise only what the code delivers, namely coverage from the load's return onward. The doc fix is small and honest; the restructure closes the window but is worth weighing against how likely a per-format loader is to panic mid-load in the first place.
- Dominant language
- Rust
- Stars
- 2
- Forks
- 2
- Avg merge
- 12h 2m
- Merged PRs (30d)
- 60
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from tableau/hyper-api-rust
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
tableau/hyper-api-rust#294 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
tableau/hyper-api-rust#311 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
tableau/hyper-api-rust#305 ·
-
Windows Named Pipe: verify DACL denies other users, and measure read-path perf for MCP workloads Open
Difficulty 4/5 3-5 days Newbie friendliness 38/100
tableau/hyper-api-rust#302 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 72/100
tableau/hyper-api-rust#300 ·
All issues in tableau/hyper-api-rust
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
state:needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
zed-industries/zed#64680 · 2 comments ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
RustPython/RustPython#8802 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
TheLarkInn/aipm#2390 ·