fix: preserve char(n) space padding in snapshot/subset queries

[alco]

Summary

• Fixed char(n) (bpchar) column values being trimmed of trailing spaces in snapshot and subset queries
• The ::text cast in pg_cast_column_to_text/1 was stripping space padding from char(n) columns, causing inconsistency with values from PG replication which correctly preserve padding
• Uses pg_typeof() at runtime to detect bpchar columns and concat() to preserve padding without trimming

Test plan

• Added test with CHAR(8) PK and CHAR(10) column verifying space padding is preserved
• Test includes NULL char(n) column to verify NULL handling is correct
• All existing querying tests pass (15/15)
• All shapes tests pass (501/501)
• All plug/router tests pass (131/131)

Fixes #4039

🤖 Generated with Claude Code

[claude]

Claude Code Review

Summary

The PR is nearly ready. The Important issue from previous reviews (.agent-tasks/ files in git) has been resolved — the files were removed in a new commit. Two minor suggestions remain unaddressed but are non-blocking.

What's Working Well

• .agent-tasks/ files removed: The new "Remove agent task files" commit deletes all 5 ephemeral agent working files from the branch. The PR diff now cleanly touches only the 3 expected files: querying.ex, querying_test.exs, and the changeset.
• All previously resolved issues remain resolved: blank line formatting, query_move_in test, pk-less table test, NULL in char(n) coverage — all still in good shape.
• Coverage is solid: Codecov reports all modified lines covered, and project coverage improved by ~3%.

Issues Found

Important (Should Fix)

None remaining.

Suggestions (Nice to Have)

1. .agent-tasks/ not added to .gitignore (partially addressed)

The files are gone from this branch, but .agent-tasks/ is not listed in .gitignore. Future agent-assisted PRs could commit these files again inadvertently. A one-line addition to .gitignore would prevent recurrence — can be done in a follow-up if preferred.

2. CHAR(1) / bare CHAR variant not tested (unchanged from previous reviews)

CHAR without a length is char(1) in PostgreSQL — internal type is still bpchar so the fix handles it, but it remains undocumented by test. A single additional row in the existing test would close this gap.

3. char(n)[] array follow-up issue not filed (unchanged from previous reviews)

The open-questions file acknowledged this limitation. No tracking issue has been filed. Low priority, but worth filing before the PR merges so it isn't forgotten.

Issue Conformance

No change from previous reviews. Implementation directly addresses the char(n) key/value padding inconsistency in #4039. Boolean inconsistency ("t"/"f" vs "true"/"false") remains out of scope and correctly so.

Previous Review Status

Issue	Status
.agent-tasks/ files committed to repo (Important)	Resolved — files deleted in new commit
Missing blank line after pg_cast_column_to_text/1 (Important)	Resolved (iteration 2)
char(n)[] follow-up issue suggested (Suggestion)	Not addressed
query_move_in test missing (Suggestion)	Resolved (iteration 2)
pk-less table / NULL in char(n) (Suggestion)	Resolved (iteration 2)
CHAR(1) / bare CHAR test (Suggestion)	Not addressed
.agent-tasks/ in .gitignore (Suggestion)	Not addressed

---

Review iteration: 3 | 2026-03-27

[claude]

test

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.67%. Comparing base (9335ae6) to head (e41ff98).
⚠️ Report is 16 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4044      +/-   ##
==========================================
+ Coverage   85.78%   88.67%   +2.88%     
==========================================
  Files          51       25      -26     
  Lines        3399     2438     -961     
  Branches      611      612       +1     
==========================================
- Hits         2916     2162     -754     
+ Misses        481      274     -207     
  Partials        2        2              

Flag	Coverage Δ
elixir	?
elixir-client	?
packages/experimental	87.73% <ø> (ø)
packages/react-hooks	86.48% <ø> (ø)
packages/start	82.83% <ø> (ø)
packages/typescript-client	93.81% <ø> (ø)
packages/y-electric	56.05% <ø> (ø)
typescript	88.67% <ø> (ø)
unit-tests	88.67% <ø> (+2.88%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

Summary

This PR fixes a real inconsistency bug: CHAR(n) (bpchar) column values in snapshot and subset queries were having their trailing space padding stripped by PostgreSQL's ::text cast, while the replication path preserved it. The fix is surgical — a single private function change in querying.ex using a runtime pg_typeof() CASE expression. The approach is correct and well-reasoned.

What's Working Well

• Minimal blast radius: Changing pg_cast_column_to_text/1 alone covers all four affected call paths (value serialization, PK key building, both make_tags paths) without touching the Shape struct, its serialization, or any other module.
• NULL handling is correct and necessary: The explicit WHEN col IS NULL THEN NULL::text guard is load-bearing — concat(NULL, '') = '' in PostgreSQL, which would silently turn NULL char(n) columns into empty strings. The test with a NULL name column verifies this.
• Changeset is present and correctly uses @core/sync-service (the actual package name).
• Issue is clearly linked and the PR description accurately explains root cause, fix strategy, and discarded alternatives.

---

Issues Found

Important (Should Fix)

1. .agent-tasks/ directory committed to repo

Files: .agent-tasks/2026-03-23--13--char-padding-bug/{plan,task,progress,review-round-1,open-questions}.md

The PR includes 5 agent working files (planning notes, progress logs, self-review) as tracked repository files. These are ephemeral task artifacts — they don't belong in git history and .agent-tasks/ is not in .gitignore. They add noise to git log and git blame and will accumulate with every future agent task.

Suggested fix: Either add .agent-tasks/ to .gitignore and remove these files from the branch, or move the agent working directory outside the repo root (e.g., ~/agents/). The .agent-tasks/ content is useful for the agent's workflow but has no value to future code readers.

2. Missing blank line between pg_cast_column_to_text/1 and pg_escape_string_for_json/1

File: packages/sync-service/lib/electric/shapes/querying.ex:286

The closing end of pg_cast_column_to_text/1 is immediately followed by defp pg_escape_string_for_json(str) with no blank line. All other adjacent defp pairs in this file have a blank line separator. mix format will not catch this since it's a style preference not enforced by the formatter, but it's inconsistent.

---

Suggestions (Nice to Have)

3. char(n)[] array columns — unhandled, worth a follow-up issue

A CHAR(n)[] column has pg_typeof() = character[], which does not match 'character'::regtype. Those elements fall through to ::text and still get trimmed. This is the same class of inconsistency as the original bug, just for arrays.

The open-questions file acknowledges this but there's no linked follow-up issue. Given char(n)[] arrays are uncommon in practice, a follow-up issue (not blocking this PR) would be appropriate to track.

4. Test: query_move_in and query_subset not covered for char(n)

The new test only exercises stream_initial_data. The fix applies equally to query_move_in (via join_primary_keys) and query_subset, but a char(n) PK in a subquery join scenario would give higher confidence. The plan mentioned this but it was skipped. Low priority since the same function is exercised by both paths.

5. Test: bare CHAR / CHAR(1) variants

CHAR without a length is char(1) in PostgreSQL and its internal type is still bpchar, so the fix handles it. A quick additional row in the test with a CHAR(1) column would document this explicitly.

---

Issue Conformance

The linked issue (#4039) clearly describes the bug with reproduction steps and actual vs. expected behavior. The implementation directly addresses the char(n) key/value padding inconsistency between changes_only and full/subset responses.

Note: the issue also mentions booleans being represented as "t"/"f" in replication vs "true"/"false" in queries. This PR does not address the boolean inconsistency — the concat() approach was specifically chosen to avoid breaking booleans (which concat(bool, '') = 't'/'f'), preserving the query path's correct true/false output. This is the right call, but the boolean discrepancy in the issue remains open. The PR description only claims to fix the char(n) padding issue, so this is not a gap in this PR — just worth noting in case a follow-up is expected.

Previous Review Status

No previous Claude review exists for this PR.

---

Review iteration: 1 | 2026-03-23

[netlify]

✅ Deploy Preview for electric-next ready!

Name	Link
🔨 Latest commit	82251ed
🔍 Latest deploy log	https://app.netlify.com/projects/electric-next/deploys/69c684284f828c0009cd8a78
😎 Deploy Preview	https://deploy-preview-4044--electric-next.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[magnetised]

Nice