-
Notifications
You must be signed in to change notification settings - Fork 37
Simple show tables
statement not cast correctly by default
#165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The problem seems two-fold:
|
Opened an exploratory PR #166 to address the first issue. |
For the second issue I believe you already can start doing: import { connect, cast, type Cast } from '@planetscale/database';
const inflate: Cast = (field, value) => {
if (field.type === 'VARBINARY' && value instanceof Uint8Array) {
return new TextDecoder().decode(value as unknown as Uint8Array);
}
return cast(field, value);
}
const conn = connect({ url: DATABASE_URL, cast: inflate }); |
I bet this can be done a bit more intelligently on BINARY/VARBINARY specifically by also starting to check in the |
Thanks for reporting the bug @cdcarson, we were able to use the |
@ayrton @mattrobenolt Thanks for the quick response and your work. Unfortunately, I found at least one other query where things that should be I have a theory about #161 and #164, possibly making all this go away. (Also possibly entirely wrong.) I'll get back to you on that. Here's the offending case. -- table def shouldn't matter, but for completeness...
CREATE TABLE `User` (
`id` bigint unsigned NOT NULL AUTO_INCREMENT,
`createdAt` datetime(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3),
PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
SHOW FULL COLUMNS FROM User; This produces arrays for {
fields: [
{
name: 'Field',
type: 'VARCHAR',
table: 'COLUMNS',
orgName: 'Field',
columnLength: 256,
charset: 255
},
{
name: 'Type',
type: 'BLOB',
table: 'COLUMNS',
orgTable: 'columns',
orgName: 'Type',
columnLength: 67108860,
charset: 255,
flags: 4241
},
{
name: 'Collation',
type: 'VARCHAR',
table: 'COLUMNS',
orgName: 'Collation',
columnLength: 256,
charset: 255
},
{
name: 'Null',
type: 'VARCHAR',
table: 'COLUMNS',
orgName: 'Null',
columnLength: 12,
charset: 255,
flags: 1
},
{
name: 'Key',
type: 'BINARY',
table: 'COLUMNS',
orgTable: 'columns',
orgName: 'Key',
columnLength: 12,
charset: 255,
flags: 4481
},
{
name: 'Default',
type: 'BLOB',
table: 'COLUMNS',
orgTable: 'columns',
orgName: 'Default',
columnLength: 262140,
charset: 255,
flags: 144
},
{
name: 'Extra',
type: 'VARCHAR',
table: 'COLUMNS',
orgName: 'Extra',
columnLength: 1024,
charset: 255
},
{
name: 'Privileges',
type: 'VARCHAR',
table: 'COLUMNS',
orgName: 'Privileges',
columnLength: 616,
charset: 255
},
{
name: 'Comment',
type: 'BLOB',
table: 'COLUMNS',
orgName: 'Comment',
columnLength: 24576,
charset: 255,
flags: 145
}
],
rows: [
{
Field: 'id',
Type: [Uint8Array],
Collation: null,
Null: 'NO',
Key: 'PRI',
Default: null,
Extra: 'auto_increment',
Privileges: 'select,insert,update,references',
Comment: Uint8Array(0) []
},
{
Field: 'createdAt',
Type: [Uint8Array],
Collation: null,
Null: 'NO',
Key: '',
Default: [Uint8Array],
Extra: 'DEFAULT_GENERATED',
Privileges: 'select,insert,update,references',
Comment: Uint8Array(0) []
}
],
} This specific case may be a simple fix, just put 'BLOB' through your Lines 408 to 414 in 0152910
Thanks again! |
That does appear true. I thought BLOB was reserved for exclusively binary only data to not even be attempted to be interpreted as text, but I guess I was wrong. This seems reasonable to me to trust the flags as authority. This whole chunk should probably be redone a bit in light of all this. |
What's interesting is that the BLOB also contains a charset. I definitely thought they explicitly did not. I wonder if this is potentially a deviation of what Vitess does. In any case, I think it's fair to just look at the flags. |
My "theory" is that the library doesn't really need to convert to an array. It's real simple to convert a buffer (say read from a file) to an encoded string, rather than passing the buffer. There may be downsides, but I think it's reasonable. Working example repo using 1.14 of this lib and base64 encoding: https://github.com/cdcarson/pscale-text-encode-reproduction/blob/main/src/case-1.ts |
SHOW CHARACTER SET LIKE 'binary' I think this is the default 'binary' pseudo character set. I don't think you can set it on a column. |
@cdcarson thank you, we'll have a closer look |
I believe this is related to PR #164.
Given this...
... I get this....
Doing this...
...works, but (a) it's mighty inconvenient typescript (
as unknown
etc,) (b) it seems like it'd likely produce similar unwanted behavior in other off-the-cuff queries, and (c) it at least requires the shape ofcast
to be changed tostring|null|Uint8Array
.Not sure if this is possible, but it'd be simpler to go back to
string|null
, and leave it up to userland or ORMs (Prisma filed the original issue) to deal with. I'm aware that it might be more complicated than this, but one can turn a string into a UintArray ...Thanks!
The text was updated successfully, but these errors were encountered: