[Mysql] Fix binary/varbinary column types as Buffer#5800
Open
ThaiTrevor wants to merge 2 commits into
Open
Conversation
Closes drizzle-team#1188 mysql2 returns BINARY/VARBINARY columns as Node Buffer, but drizzle-orm inferred them as `string` and lossily called `.toString()` on the Buffer in `mapFromDriverValue`, corrupting arbitrary binary content (e.g. a UUID stored as `binary(16)`). Align the column data type with the driver: - `dataType` is now `'buffer'` (same convention as `sqlite-core` blob in buffer mode). - The inferred `data` type is `Buffer`; `driverParam` accepts `Buffer | string` since mysql2 also accepts hex strings. - `mapFromDriverValue` returns the `Buffer` as-is (or wraps a `Uint8Array`/`string` via `Buffer.from`). Updates the type-test default values and the mysql "all types" integration test expectations to use `Buffer` accordingly.
Author
|
Quick check-in — anything I can adjust to make this easier to review? Happy to break it down, add tests, or revise the approach. Thanks! |
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.
Summary
Closes #1188.
mysql2returnsBINARY/VARBINARYcolumns as a NodeBuffer, butdrizzle-ormtyped the column data asstringand lossily calledBuffer.prototype.toString()on the driver value inmapFromDriverValue. That silently corrupted any non-UTF-8 binary content (e.g. a UUID stored asbinary(16), or a hash stored asvarbinary(32)).This aligns the column data type with what the driver actually returns:
dataTypeis now'buffer'(same convention assqlite-coreblob in buffer mode).datatype isBuffer;driverParamacceptsBuffer | stringsincemysql2also accepts hex strings as input.mapFromDriverValuereturns theBufferas-is (or wraps aUint8Array/stringviaBuffer.from) instead of stringifying it.Files changed
drizzle-orm/src/mysql-core/columns/binary.tsdrizzle-orm/src/mysql-core/columns/varbinary.tsdrizzle-orm/type-tests/mysql/tables.ts— update.default('')to.default(Buffer.from(''))so the existing type test continues to compile against the new type.integration-tests/tests/mysql/mysql-common.ts— update the mysql "all types" suite to insert/expectBufferfor thebinaryandvarbincolumns.Note for maintainers
This is a typed-API change for users who relied on the previous
stringinference, but the previous runtime behaviour was already broken for true binary payloads (silent UTF-8 conversion), so this aligns the types with the driver's real output and lets users store/retrieve binary data safely.The parallel files in
singlestore-core(columns/binary.ts,columns/varbinary.ts) share the same broken pattern; they are intentionally left untouched here to keep this PR scoped to the issue, but happy to apply the same fix in a follow-up if you'd like.Test plan
pnpm --filter drizzle-orm test:types— type-tests indrizzle-orm/type-tests/mysql/tables.tsnow assertBufferinstead ofstringforbinary/varbinary.cd integration-tests && pnpm test— mysql "all types" test inserts aBufferand asserts it is returned asBuffer.