go-sqlite3
go-sqlite3 copied to clipboard
Add Blob I/O
Incremental Blob I/O
This PR adds support for Incremental Blob I/O.
Notes
- Modeled after backup using a driver connection.
- The interfaces io.Reader and io.Closer have been implemented.
- Related to issue #239
If this PR makes sense, I'll enhance it with additional io interfaces.
interesting.
- [x] I guess it depends on #1079 to have successful checks.
I have implemented write and seek support. As far as I'm concerned, this pull request is ready for further review. Thanks so far.
I was tempted to implement the io.ReaderAt and io.WriterAt interfaces as well, but clients can do that themselves by combining io.Reader (or io.Writer) and io.Seeker:
type Foo struct {
io.ReadSeeker
}
func (f *Foo) ReadAt(p []byte, off int64) (int, error) {
_, err := f.Seek(off, io.SeekStart)
if err != nil {
return 0, err
}
n, err := f.Read(p)
return n, err
}
By the way, I'm ok with you making your desired changes so we can close this. That might be more efficient? If you agree?
Codecov Report
Base: 45.96% // Head: 47.05% // Increases project coverage by +1.09% :tada:
Coverage data is based on head (
fe0ed2e) compared to base (da62659). Patch coverage: 69.84% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## master #1083 +/- ##
==========================================
+ Coverage 45.96% 47.05% +1.09%
==========================================
Files 11 12 +1
Lines 1499 1562 +63
==========================================
+ Hits 689 735 +46
- Misses 670 680 +10
- Partials 140 147 +7
| Impacted Files | Coverage Δ | |
|---|---|---|
| blob_io.go | 69.84% <69.84%> (ø) |
|
| sqlite3.go | 52.86% <0.00%> (+0.22%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@rittneje are you okay to merge?
The Linux builds with libsqlite3 fail with:
=== RUN TestBlobRead
blob_io_test.go:46: no such vfs: memdb
--- FAIL: TestBlobRead (0.00s)
I don't know how to fix this.
Hello!
The Linux builds with libsqlite3 fail with:
=== RUN TestBlobRead blob_io_test.go:46: no such vfs: memdb --- FAIL: TestBlobRead (0.00s)I don't know how to fix this.
Problem is like this:
- github actions uses ubuntu-latest, which is focal release, version 20.04 (stable release) at time of writing this answer
- default version of sqlite3 in this release of ubuntu is 3.31.0
- it's compiled without SQLITE_ENABLE_DESERIALIZE which is required to enable memdb vfs in sqlite verison < 3.36.0
So - vfs=memdb will not work in ubuntu focal. You would need to recompile libsqlite3 deb package and add SQLITE_ENABLE_DESERIALIZE option to make. Without it memdb is just not there. And package libsqlite3-dev do not consist c code of sqlite3 - it's .a and .so files, already precompiled.
This behaviour was changed in sqlite 3.36.0 - SQLITE_ENABLE_DESERIALIZE is defined by default, you need to explicity disable it by defining SQLITE_OMIT_DESERIALIZE option during sqlite compilation.
So, there are imho there are two solution of this problem:
- use connection string: "file:foobar?mode=memory&cache=shared" - maybe only if version of sqlite3 is < 3.36.0
- or use connection string ":memory:" and set db.SetMaxOpenConns(1) (mayb followed by db.Ping() to create conn?) to limit all access to the same memory database on all versions of sqlite, including sqlite3
This problem will disappear after ubuntu will release next LTS version, and github action will change ubuntu-latest to this LTS version - there should be at least sqlite3 3.37.0 there.
sugested fix (will work with any version of sqlite):
diff --git a/blob_io_test.go b/blob_io_test.go
index 3e6fb91..b98df8d 100644
--- a/blob_io_test.go
+++ b/blob_io_test.go
@@ -25,12 +25,14 @@ var _ io.Closer = &SQLiteBlob{}
type driverConnCallback func(*testing.T, *SQLiteConn)
func blobTestData(t *testing.T, dbname string, rowid int64, blob []byte, c driverConnCallback) {
- db, err := sql.Open("sqlite3", "file:/"+dbname+"?vfs=memdb")
+ db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
t.Fatal(err)
}
defer db.Close()
+ db.SetMaxOpenConns(1)
+
// Test data
query := `
CREATE TABLE data (
@rittneje do you agree with the proposed fix?
@joriszwart In this particular case :memory: should work. However, the need to throttle the connection pool to one can cause some issues in general. If you make that change, you should also add comments explaining it is specifically to support older SQLite versions and developers should always use file:/<name>?vfs=memdb instead whenever possible. (Developers may reference these test cases to see how to use the blob i/o feature.)
Really I think we need a better approach for dealing with testing libsqlite3 in general. But that is an issue beyond the scope of your changes.
Any updates on this? It looks very interesting and enables "streaming" to and from the database! 👍
@rittneje Is there anything I can do to get this approved?
@joriszwart I am away from my dev machine or I'd just add those two len checks myself. They should be pretty simple to add.
@joriszwart I am away from my dev machine or I'd just add those two
lenchecks myself. They should be pretty simple to add.
Can you add them?
Anyone else? @graf0 @pokstad @jlelse @lezhnev74 @mitar?
@rittneje can I leave the suggested changes to you? That would make this process more efficient.
A shame this was abandoned. Thanks for trying @joriszwart !