Implement BLOB reading for sqlite by matthiasbeyer · Pull Request #4994 · diesel-rs/diesel

matthiasbeyer · 2026-03-02T09:42:04Z

This patch implements BLOB reading via the sqlite3_blob_read() API.

--

We (@weiznich and I) talked about this on mastodon.

This is a first proposal to see what you think of it.

This patch would not have been possible without the tremendous help from @TheNeikos who is mentioned as a co-author in the patch. Thank you very much for your help ❤️

diesel/src/sqlite/connection/raw.rs

matthiasbeyer · 2026-03-02T10:04:00Z

diesel/src/sqlite/connection/mod.rs

        NonZeroI64::new(self.raw_connection.last_insert_rowid())
    }

+    pub fn get_blob<'conn, 'query, U>(&'conn self, primary_key: i64) -> Result<sqlite_blob::SqliteBlob<'conn>, Error>


Documentation missing here, yes.

This patch implements BLOB reading via the sqlite3_blob_read() API. Co-authored-by: Marcel Müller <neikos@neikos.email> Signed-off-by: Marcel Müller <neikos@neikos.email> Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

weiznich · 2026-03-02T13:42:01Z

Thanks for working on this. I (or someone else) will try to have a look at the implementation in the next days/weeks.

cc @LucaCappelletti94 as he has done quite a lot of Sqlite api additions in the last weeks and might also help here

TheNeikos · 2026-03-02T14:22:17Z

One thing I'd like to mention wrt. Blobs in SQLITE and the current interface is that one cannot hold Blobs while executing any other command.

This is fine and the easiest way from a technical standpoint. However I feel it does preclude from more interesting applications like having proper-streaming or similar backed by a sqlite database. The docs seem fairly clear on this:

sqlite_close specifically mentions BLOB handles:

If the database connection is associated with unfinalized prepared statements, BLOB handlers, and/or unfinished sqlite3_backup objects then sqlite3_close() will leave the database connection open and return SQLITE_BUSY.
(Emphasis mine)

As well as in blob_open:

If the row that a BLOB handle points to is modified by an UPDATE, DELETE, or by ON CONFLICT side-effects then the BLOB handle is marked as "expired". This is true if any column of the row is changed, even a column other than the one the BLOB handle is open on. Calls to sqlite3_blob_read() and sqlite3_blob_write() for an expired BLOB handle fail with a return code of SQLITE_ABORT. Changes written into a BLOB prior to the BLOB expiring are not rolled back by the expiration of the BLOB. Such changes will eventually commit if the transaction continues to completion.

This means that the lifetime of a Blob is tied to a database, but not in a way that Rust can properly check. The current interface also means that users cannot move a blob handle from where it was created (due to blob being bound to the lifetime of the connection).

As far as I can tell it is safe to not have the lifetime. However, this would introduce the hazard of panicking when dropping the SQLite connection while having any blobs open (as it would return SQLITE_BUSY on close).

None of these are real 'problems' IMO, just decisions to be made one way or another.

I believe a potential path forward could be something akin to a detach method on the blob that would make it 'static and put the onus on the user to understand what this entails.

weiznich

Thanks for working on this. Overall this looks like a good start already. I've left a bunch of comments around the API design I would like to discuss and potentially address before merging.

I still need to look a the tests to decide if we want to add more cases.

Finally I think that's a feature that deserves a changelog entry, so please at it to the top level Changelog.md file.

diesel/src/sqlite/connection/raw.rs

diesel/src/sqlite/connection/sqlite_blob.rs

weiznich · 2026-03-05T14:39:13Z

diesel/src/sqlite/connection/sqlite_blob.rs

+    }
+}
+
+impl SqliteBlob<'_> {


We likely also want to implement Write?

That doesn't necessarily need to be done in this pr, but the design should allow it.

Yes, I would like to include that later down the road!

weiznich · 2026-03-05T14:41:13Z

diesel/src/sqlite/connection/raw.rs

+                table_name.as_c_str().as_ptr(),
+                column_name.as_c_str().as_ptr(),
+                pkey,
+                0,


We need a parameter to control this flag. Maybe even as compile time flag (generic parameter?)

Otherwise an enum could also be fine. For now that could just have a read variant and be marked as #[non_exhaustive]

I see what you mean here. If you don't mind, I would include Write functionality in this very same PR later down the road and do that bit of refactoring for a generic parameter (or something like that) then.

But first I would like to get the Read functionality in a shape that's ACK'd by you or other diesel maintainers!

It's fine to also postpone the write functionality to a later PR.

This comment was mostly about API design.

matthiasbeyer · 2026-03-06T08:04:17Z

I pushed a fixup to address a number of your review points, where I did not have any questions! Also included some SAFETY comments in the codebase now.

LucaCappelletti94 · 2026-03-06T12:31:55Z

diesel/src/sqlite/connection/mod.rs

        NonZeroI64::new(self.raw_connection.last_insert_rowid())
    }

+    pub fn get_blob<'conn, 'query, U>(


Undocumented public method, maybe a documentation with doctests would be nice to have?

weiznich

Thanks for the update and sorry for taking that long to review it :(

The API design now mostly looks good to me, beside the minor note about close. I would be happy to merge this with the remaining minor things resolved, a bit of documentation + a changelog entry added.

weiznich · 2026-03-20T13:30:35Z

diesel/src/sqlite/connection/mod.rs

    ffi::code_to_str(err_code)
 }

+#[doc(hidden)]


Minor nit: I usually put these traits in a local mod private {} and then reexport them as crate local via pub(crate) use self::private::HasDatabaseAndTableName; in the parent scope. That ensures that they are really not usable by downstream crates.

weiznich · 2026-03-20T13:31:51Z

diesel/src/sqlite/connection/raw.rs

+                table_name.as_c_str().as_ptr(),
+                column_name.as_c_str().as_ptr(),
+                pkey,
+                0,


It's fine to also postpone the write functionality to a later PR.

This comment was mostly about API design.

weiznich · 2026-03-20T13:33:50Z

diesel/src/sqlite/connection/sqlite_blob.rs

+    /// Using the handle after calling this function is an error if not otherwise stated.
+    pub fn close(&mut self) -> Result<(), crate::result::Error> {


Maybe rather expose this publically as consuming method (so pub fn clone(self)) and only internally have a fn close_intern(&mut self) method that can be shared between Drop and this function?

Seems like an easy way to remove this potential error path

diesel/src/sqlite/connection/sqlite_blob.rs

matthiasbeyer force-pushed the sqlite-blob-reading branch 3 times, most recently from 8d1cc15 to 90e35aa Compare March 2, 2026 10:01

matthiasbeyer commented Mar 2, 2026

View reviewed changes

diesel/src/sqlite/connection/raw.rs Outdated Show resolved Hide resolved

matthiasbeyer commented Mar 2, 2026

View reviewed changes

Implement BLOB reading for sqlite

ebed4d6

This patch implements BLOB reading via the sqlite3_blob_read() API. Co-authored-by: Marcel Müller <neikos@neikos.email> Signed-off-by: Marcel Müller <neikos@neikos.email> Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

matthiasbeyer force-pushed the sqlite-blob-reading branch from 90e35aa to ebed4d6 Compare March 2, 2026 10:05

weiznich requested a review from a team March 2, 2026 13:40

weiznich reviewed Mar 5, 2026

View reviewed changes

fixup! Implement BLOB reading for sqlite

7d37d05

matthiasbeyer added 3 commits March 6, 2026 09:24

fixup! Implement BLOB reading for sqlite

f6c536c

fixup! 7d37d05

98517a8

fixup! 7d37d05

d2cee10

LucaCappelletti94 suggested changes Mar 6, 2026

View reviewed changes

weiznich reviewed Mar 20, 2026

View reviewed changes

		/// Using the handle after calling this function is an error if not otherwise stated.
		pub fn close(&mut self) -> Result<(), crate::result::Error> {

Uh oh!

Conversation

matthiasbeyer commented Mar 2, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weiznich commented Mar 2, 2026

Uh oh!

TheNeikos commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weiznich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthiasbeyer commented Mar 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weiznich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TheNeikos commented Mar 2, 2026 •

edited

Loading