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

Simple read-only SQLite databases embedded in Go executables #1188

Closed
wants to merge 1 commit into from
Closed

Simple read-only SQLite databases embedded in Go executables #1188

wants to merge 1 commit into from

Conversation

dmarker
Copy link

@dmarker dmarker commented Sep 4, 2023

The deserialize support that was added for #1089 doesn't quite address issue #968. But it comes very close. With this code you can basically follow along with a SO from gizlu. He even provided a working C example.

There is a limit due to SQLite only accepting one "mode=" in the DSN string. The C library would have to provide a Read-only memory option to have a named schema. Right now only "main" can be used, therefore only one embedded DB at a time can be used.

@rittneje
Copy link
Collaborator

rittneje commented Sep 4, 2023

It is unclear to me whether this could run afoul of the rules of cgo. So it might be good to explicitly pin the memory first. Once you do that, it means this new method is not specific to embedded databases per se, but rather can be used as long as you guarantee that the byte slice in question will not be modified.

You will, however, need to determine when we can safely unpin the memory.

@dmarker
Copy link
Author

dmarker commented Sep 4, 2023

I'm relying on go GC never collecting a global since it is always reachable and I'm probably naive in believing it won't move it either. It seems it wouldn't so long as their value doesn't alter, but I will need to research that.

It has been a while since I've written go code. You can tell as I mention in the test I don't make the byte array const (you can't in go!).

I am certain you can only have a global for embed. The name DeserializeEmbedded is at least a hint that it really does in fact need to be embedded (actually global). I'll need to look more into runtime.Pinned to see if that will give the necessary guarantee. I would mostly be interested in sticking to the embed case so I don't have to worry about unpinning later.

@rittneje
Copy link
Collaborator

rittneje commented Sep 5, 2023

I'm probably naive in believing it won't move it either

The Go authors have reserved the right to implement a moving garbage collector in the future, which theoretically could affect global variables. Probably for the best to account for that now to avoid issues down the line.

@dmarker
Copy link
Author

dmarker commented Sep 5, 2023

I don't think this PR is worth bumping go.mod to go1.21 which is where it looks like the new runtime.Pinned API is stable. Pinning would also protect against reassigning the embed variable which would lead to current GC reclaiming the memory which is just as bad as a move by a future GC.

I am fine just patching my go-sqlite for the project I intend to use this feature with for now, and it can be revisited when go.mod requires go1.21 or higher version.

@rittneje
Copy link
Collaborator

rittneje commented Sep 5, 2023

You should be able to use built in build tags to only support the new method in 1.21 instead of updating go.mod.

@dmarker
Copy link
Author

dmarker commented Sep 5, 2023

I'll update to do that. Looks like the tag is for the whole file so I have some refactoring as well as retesting. Thank you for your suggestion!

@dmarker
Copy link
Author

dmarker commented Sep 14, 2023

Sorry about the delay in refactor, I'm in the middle of moving. Ready for you to take a look now.

@dmarker dmarker closed this Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants