fix(schema): encode msgpack fields with the dialect binary literal (#1219) - #1421
Open
ChrisJr404 wants to merge 2 commits into
Open
fix(schema): encode msgpack fields with the dialect binary literal (#1219)#1421ChrisJr404 wants to merge 2 commits into
ChrisJr404 wants to merge 2 commits into
Conversation
The msgpack field appender always emitted the encoded value using
PostgreSQL's '\x...' bytea literal, regardless of the target dialect.
SQLite and MySQL do not recognize that syntax and stored the payload
verbatim as text, so reading the row back failed with a msgpack decode
error (e.g. "unexpected code=5c decoding map length").
Encode into a buffer and delegate to Dialect.AppendBytes so each dialect
emits its own binary literal ('\x...' for PostgreSQL, X'...' for SQLite
and MySQL). The PostgreSQL output is byte-for-byte identical to before.
Fixes uptrace#1219
Emitting the dialect binary literal was only half of it. The column type was still derived from the field's Go type, so a msgpack struct became a JSON/text column (JSONB on Postgres, JSON on MySQL, NVARCHAR on MSSQL). Inserting the binary literal into those columns fails on every strict database, which is why TestMsgpackRoundTrip was red for pg, mysql, mariadb and mssql. Force msgpack fields to the Blob type so each dialect creates a real binary column (bytea, blob, varbinary) that matches the literal we append. Sqlite already passed and still does.
Author
|
The round-trip test was still failing on pg, mysql, mariadb and mssql because the value literal was only half the problem. A msgpack struct was still mapped to a JSON/text column, so inserting the binary literal got rejected. Pushed a follow-up that forces msgpack fields to a Blob column (bytea, blob, varbinary), so the column type matches the literal. Verified the sqlite round-trip passes locally and confirmed the generated DDL is a binary column on all four dialects. |
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.
Describe the pull request
Fields tagged with
bun:",msgpack"did not round-trip on SQLite (and MySQL) — only on PostgreSQL.appendMsgpackencoded the value straight throughinternal.HexEncoder, which always emits PostgreSQL's'\x...'bytea literal. SQLite and MySQL don't recognize that syntax, so the payload was stored verbatim as text and reading the row back failed with a msgpack decode error:(
0x5cis the leading\of the\xprefix that got stored as data.)The fix encodes the msgpack payload into a buffer and delegates to
Dialect.AppendBytes, the same routine bun already uses for regular[]bytecolumns. Each dialect now emits its own binary literal —'\x...'for PostgreSQL,X'...'for SQLite and MySQL. The PostgreSQL output is byte-for-byte identical to before, so existing behavior is unchanged.internal.HexEncoderwas the only consumer of that code and is now unused, so it's removed.Link to the issue: #1219
Tests
internal/dbtest:TestMsgpackRoundTrip— create/insert/select round-trip of a,msgpackfield, run for every dialect viatestEachDB(verified green on SQLite locally).schema:TestAppendMsgpack_DialectSpecificLiteral— asserts the appended literal matches the dialect'sAppendBytesand that PostgreSQL still produces the'\x...'form.Checklist