-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add libSQL Store support #1
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The prepared SQL statements here are MySQL specific, and the Golang MySQL driver I'm using (though seemingly fairly official) leaves a lot to be desired. Benchmark needs to be modified: each time we run the benchmark, we need to generate a *fresh* store (out of timer) so that the times will converge.
Why bad? Testing an in-memory SQL driver isn't really fair; it's unclear how performant it is vs. actual MySQL. Moreover, dynamics in practice are a little different: if a store is being loaded to serve the request, the whole file has to be parsed; in this test, the file is parsed once and then overwritten. That might also explain why the file perf doesn't scale with initial file size: I should close and reopen it. Anyway, the store benchmark concept was interesting; I learned something here.
This reverts commit 367d53a.
Could also consider e.g. a Turso store, but the supported SQL syntax is slightly different (SQLite).
Tested with a local file. The "connection-string" name is long and clumsy, and maybe specifically correct for Postgres; "URL" feels too confusing. Maybe a "db" argument? I doubt I'll add support for another DB anytime soon.
This doesn't strictly obviate cmd/tir/cmd/migrate, which was designed to move TIR records from an HTML file (old form) into a tiir store. That's still relevant, but it's unnecessarily messy if your texts are already in a store. I'll consider consolidating.
s/init/prepare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new store supports local sqlite file-databases — probably a more efficient alternative to my homebrewed
store.File
— and hosted databases by Turso.cmd/migrate
for migrating between tiir stores; see ea8cec4 message.To do
Decide whether to move to Turso or a sqlite DB local to fly.dev for my "prod" tir env.Look out for resolution or clarification on Can'tScan
timestamps written without locations tursodatabase/libsql-client-go#79.