fix: pandas bulk path falls back to SQL on any failure (poisoned-buffer 500 loop)#34
Merged
Conversation
…ilege errors
Observed in production: on a session whose backing protocol cannot run PUT
file transfers, write_pandas dies inside file_transfer_agent with
KeyError('command') — not a privilege error — so the fallback never engaged,
the exception propagated, and because the buffer was never cleared, every
subsequent flush-before-read re-fired it (all reads 500 on that process).
The pandas path is an optimization, not a correctness requirement: any
failure now logs a warning, best-effort drops the staging table, and falls
back to the DDL-free SQL path (idempotent, carries all columns as of
v0.7.10). Regression test simulates the poisoned-buffer scenario and asserts
fallback + buffer clear + quiet second flush. Bumps version to 0.7.11.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vigneshnarayanaswamy
added a commit
that referenced
this pull request
Jul 10, 2026
…35) Blind post-merge review of #33/#34 found the composition bug: Snowflake processes backslash escape sequences inside single-quoted constants, and _esc only doubled single quotes — so any JSON payload containing an embedded double quote (serialized by json.dumps as \") or a backslash reached PARSE_JSON mangled. On the SQL fallback path (which #34 routes all non-native-transport deployments onto) that meant: INSERT fails, buffer never clears, every subsequent flush-before-read 500s — the poisoned-buffer incident, reintroduced on common data. Verified live: PARSE_JSON('{"a": "x \" y"}') errors; the doubled form round-trips. Also from the review: privilege denials (the expected steady state of least-privilege roles) short-circuit quietly again instead of warning + issuing a doomed DROP per flush; unexpected failures log with exc_info; the failure-path cleanup DROP is gone (temp tables die with the session, and it could target a same-named permanent table); row construction sits inside the try; the null-payload regression test inspected the wrong side of FROM and could never fail — fixed; snapshot-path fallback and quote/backslash round-trip tests added. Version 0.7.12. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Production incident (2026-07-10, downstream deployment): on a session whose backing protocol cannot run
PUTfile transfers,write_pandasdies insidefile_transfer_agentwithKeyError('command'). That is not a privilege error, so the v0.7.10 fallback never engaged — the exception propagated to the caller, and because the write buffer was never cleared, every subsequent flush-before-read re-fired it: one buffered write turned every read on that process into a 500.Fix: the pandas bulk path is an optimization, never a correctness requirement. Any failure now logs a warning (with exception type), best-effort drops the staging table, and returns
Falseso the DDL-free SQL path — idempotent and, as of v0.7.10, carrying all columns — handles the flush.Regression test simulates the exact scenario: pandas-capable-looking session +
write_pandasraisingKeyError('command')→ flush must not raise, SQL MERGE must run, buffer must clear, second flush must be a no-op.758 tests, ruff, mypy green. Version → 0.7.11.
🤖 Generated with Claude Code