Repository navigation
Convert latin1 VStream text to UTF-8 before JSON encoding - #159
Merged
Merged
Conversation
dtest
marked this pull request as draft
October 12, 2026 14:29
lizztheblizz
left a comment
Contributor
There was a problem hiding this comment.
Plain latin1 text comes through correctly now, nice. Two cases are still wrong, and one of them regresses incremental syncs compared to main. Exact fixes are inline. With them applied the full suite passes, and the new tests fail on the current head in exactly the two places below.
I reproduced this end to end with a real VStream from a local vttestserver (MySQL 8.0), through the copy phase and then a binlog-phase insert, and confirmed the _bin typing on MySQL 8.4.11 too:
| column | phase | main | this PR |
|---|---|---|---|
varchar latin1_swedish_ci |
both | S�o Paulo ��� |
São Paulo €‘’ |
char/varchar/text latin1_bin |
both | Kak� |
Kak� |
enum/set latin1 |
copy | Bras�lia |
Brasília |
enum/set latin1 |
binlog | Brasília |
BrasÃlia |
- Binlog ENUM/SET values get double-encoded. Since Vitess v21, vstreamer maps binlog ENUM/SET ordinals to labels taken from
information_schema.columns.column_type(buildEnumStringValue), so they arrive as UTF-8 whileField.Charsetstill says latin1. The copy phase sends raw latin1 bytes (set names 'binary'in rowstreamer). Converting both breaks values that main gets right, and a full refresh and an incremental sync emit different strings for the same row. - latin1_bin columns are skipped. MySQL sets BINARY_FLAG on
_bincollations andsqltypesmaps that to BINARY/VARBINARY/BLOB, withCharsetstill 47.isTextColumnfilters them out. Real binary columns are charset 63, which isn't inlatin1Collations, so gating on the collation alone is enough.
Not blocking, but worth deciding before merge:
- UTF-8 bytes stored in latin1 columns (an app writing UTF-8 through a latin1 connection) used to pass through as
Sãoand now come out asSão. That's exactly what MySQL returns to a utf8mb4 client, so I think the PR is right, but it's a visible change for anyone with legacy data like that. - Already-synced rows don't heal. The U+FFFD substitution is lossy, so affected streams need a full refresh once this ships. Both points are worth a line in the release notes.
- Optional: vitess v0.17.6 already has
charset.Convertplus a collation lookup. For latin1 it's byte-identical todecodeWindows1252on collations 5/8/15/47/48/49/94, and it also covers latin2/cp1250/cp1251. Collation 31 (latin1_german2_ci) is missing from v0.17.6's table, so it would need a fallback. Fine to keep the hand-rolled table.
The cp1252 table matches MySQL's latin1 for all 32 bytes in 0x80-0x9F.
Co-authored-by: Lizz <lizztheblizz@gmail.com>
Co-authored-by: Lizz <lizztheblizz@gmail.com>
Co-authored-by: Lizz <lizztheblizz@gmail.com>
Skip conversion whenever the value is already valid UTF-8, and drop isDeclaredLabel. With this, the output only differs from main where main emitted U+FFFD; anything main passed through intact is unchanged. It also fixes two cases the label check got wrong: - Binlog ENUM labels containing a quote were double-encoded. column_type stores 'café''s' and vitess unescapes it, but parseEnumOrSetValues does not, so the match failed and "café's" became "café's". - UTF-8 bytes stored in latin1 columns (written through a latin1 connection) now come through as "São" again instead of "São". The cost is latin1 text whose high bytes happen to form valid UTF-8 (e.g. "é" stored as C3 A9), which stays as main shows it. The copy phase does deliver raw latin1 bytes (rowstreamer runs set names 'binary'), so the label check was only ever needed for the binlog phase. Verified end to end against vttestserver in both phases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dtest
marked this pull request as ready for review
October 13, 2026 19:10
nickvanw
approved these changes
Oct 13, 2026
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
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.
latin1 characters become mangled when converting to utf8 in airbyte. This conversion is aimed to fix that.