Surface row codec errors instead of silently returning empty rows#120
Surface row codec errors instead of silently returning empty rows#120mweiden wants to merge 1 commit into
Conversation
encode_row/decode_row swallowed serde_json errors with
unwrap_or_default(), so a corrupted stored row decoded to an empty map
- indistinguishable from a legitimately deleted row. Corruption showed
up as silent data loss ('all columns gone') instead of an error.
Both functions now return Result. Empty input remains a valid
representation of an absent/deleted row (tombstones), but non-empty
data that fails to parse is reported as a QueryError. Read repair
logs and skips undecodable replica values rather than aborting the
whole repair pass.
https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
![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: 452d01700c
ℹ️ 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".
| if !val.is_empty() { | ||
| let map = decode_row(val.as_bytes()); | ||
| arr_rows.push(map); | ||
| arr_rows.push(decode_row(val.as_bytes())?); |
There was a problem hiding this comment.
When one replica returns the new QueryError::Codec but another replica returns a valid Meta row, merge_results records the error in last_err and then enters this rows branch, returning the decoded rows and never surfacing the corruption. In an RF>1 read where a corrupt replica is contacted but enough other replicas satisfy the read consistency, this still silently hides the row codec error that this change is intended to expose; consider prioritizing codec errors before returning merged rows.
Useful? React with 👍 / 👎.
2 participants
Problem
encode_row/decode_rowswallowed serde_json errors withunwrap_or_default(), so a corrupted stored row decoded to an empty map — indistinguishable from a legitimately deleted row. On-disk corruption surfaced as silent data loss ("all columns gone") instead of an error anyone could detect.Fix
Result<_, RowCodecError>(new error type inschema.rs).QueryErrorgains a transparentCodecvariant; the query engine and the cluster LWT paths propagate it.build_rownow returns the column map and callers encode it, so encode failures propagate there too.Tests
corrupt_row_surfaces_error_instead_of_empty_row: garbage bytes in a stored row turn a SELECT into an error, not an empty result.deleted_row_still_reads_as_absent: tombstone behavior unchanged.Found by codebase audit (medium severity: corruption masked as silent data loss).
https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
Generated by Claude Code