Skip to content
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

[FEATURE]: SQLite: Support blob SQL type in relational queries #2171

Open
Colonel-Sandvich opened this issue Apr 17, 2024 · 0 comments · May be fixed by #2173
Open

[FEATURE]: SQLite: Support blob SQL type in relational queries #2171

Colonel-Sandvich opened this issue Apr 17, 2024 · 0 comments · May be fixed by #2173
Labels
enhancement New feature or request

Comments

@Colonel-Sandvich
Copy link

Colonel-Sandvich commented Apr 17, 2024

Describe what you want

This issue has been made a few times and the conclusion is store json in text not blob type. However, my use case is storing UUIDs in blob instead of text to cut down on storage. (Maybe there's perf gains for column comparisons too, who knows)

Proposed solution:

If the column is of blob type instead of sending an invalid query, wrap the field in hex and return it as a string then convert that back into a Buffer in drizzle.

The first part seems easy,

field.getSQLType() === 'blob' ? sql.raw(`hex("${field.name}")`)

If this could be done better, let me know.

Git Diff
diff --git a/drizzle-orm/src/sqlite-core/dialect.ts b/drizzle-orm/src/sqlite-core/dialect.ts
index aa229d23..ec485a31 100644
--- a/drizzle-orm/src/sqlite-core/dialect.ts
+++ b/drizzle-orm/src/sqlite-core/dialect.ts
@@ -638,7 +638,13 @@ export abstract class SQLiteDialect {
                        let field = sql`json_array(${
                                sql.join(
                                        selection.map(({ field }) =>
-                                               is(field, SQLiteColumn) ? sql.identifier(field.name) : is(field, SQL.Aliased) ? field.sql : field
+                                               is(field, SQLiteColumn)
+                                                       ? field.getSQLType() === 'blob'
+                                                               ? sql.raw(`hex("${field.name}")`)
+                                                               : sql.identifier(field.name)
+                                                       : is(field, SQL.Aliased)
+                                                       ? field.sql
+                                                       : field
                                        ),
                                        sql`, `,
                                )

Converting this string back to a buffer seems less easy to me.

Maybe we can add:

  mapFromDriverValue(value: Buffer | string) {
    if (typeof value === "string") {
      value = hexStringToBuffer(value);
    }
    return value;
  }

To SQLiteBlobBuffer. And then value.toString() -> typeof value === "string" ? value : value.toString() in the other two blob classes.

That just leaves custom column types:

override mapFromDriverValue(value: T['driverParam']): T['data'] {
+  if (typeof value === 'string' && this.getSQLType() === 'blob') {
+    value = hexStringToBuffer(value);
+  }
  return typeof this.mapFrom === 'function' ? this.mapFrom(value) : value as T['data'];
}

Cons:
Extra overhead of converting hex strings to Buffer. This isn't done for BlobJSON or BlobBigInt but it is always done for custom blob columns and regular BlobBuffer even if the consumer then converts these to strings. So there's some compute wasted there.
Pros:
We can fetch blob type columns in relational queries!

Many thanks for reading ❤️

@Colonel-Sandvich Colonel-Sandvich added the enhancement New feature or request label Apr 17, 2024
@Colonel-Sandvich Colonel-Sandvich changed the title [FEATURE]: SQLite: Support blob SQL type in relational query [FEATURE]: SQLite: Support blob SQL type in relational queries Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant