fix: SQL fallback snapshot flush drops PAYLOAD and TAGS#33
Conversation
_flush_snapshots_sql — the DDL-free path taken whenever the role cannot CREATE TEMPORARY TABLE (least-privilege deployments: INSERT/UPDATE only) — inserted only the 7 scalar columns, silently persisting every snapshot with NULL PAYLOAD and TAGS. The pandas path carries both; the fallback must too. Payload/tags now ride the UNION ALL source as escaped JSON strings and land via PARSE_JSON in the INSERT...SELECT (expressions are not allowed in VALUES, but are in SELECT). Two regression tests: content preservation, and NULL (not 'null') for empty payloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4fc2113c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"{_esc(json.dumps(s.payload, default=str)) if s.payload else 'NULL'}, " | ||
| f"{_esc(json.dumps(s.tags, default=str)) if s.tags else 'NULL'}" |
There was a problem hiding this comment.
When the SQL fallback is used and a payload/tag string contains JSON escapes (\n, \t, \\, \uXXXX, Windows paths, regexes), _esc(json.dumps(...)) only doubles quotes but leaves backslashes inside a Snowflake single-quoted literal. Snowflake treats backslashes in such literals as escape sequences (docs), so PARSE_JSON sees different or invalid JSON (for example, an actual newline inside a JSON string) and the flush can fail or corrupt payloads; escape backslashes or use dollar-quoted/parameterized literals before parsing.
Useful? React with 👍 / 👎.
…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>
1 participant
The DDL-free SQL fallback (
_flush_snapshots_sql) — the path taken whenever the role can'tCREATE TEMPORARY TABLE, i.e. every least-privilege deployment with row-level grants only — inserted just the 7 scalar columns. Every snapshot flushed through it persisted with NULLPAYLOADandTAGS, silently dropping the event data itself (discovered ports/metadata, observation payloads, record bodies).The pandas path has always carried both columns; the fallback now matches it: payload/tags ride the
UNION ALLsource as escaped JSON strings and land viaPARSE_JSON(...)in theINSERT...SELECT(expressions are not allowed inVALUES, but are inSELECT).Regression tests: content preservation through the fallback, and SQL
NULL(not the string'null') for empty payload/tags.Found by automated review on the downstream repin (a deployment whose writer role has exactly this grant shape); verified against the wheel source before fixing.
🤖 Generated with Claude Code