Skip to content

feat: support sqlite open v2 #132

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 23, 2022
Merged

feat: support sqlite open v2 #132

merged 3 commits into from
Jul 23, 2022

Conversation

kkharji
Copy link
Owner

@kkharji kkharji commented Dec 28, 2021 •
edited
Loading

Purpose

Enable users to control how should the sqlite database opened via opts.open_mode, while enabling support for uri processing https://sqlite.org/c3ref/open.html#urifilenamesinsqlite3open

  • Fix docgen
  • Fix tests of fields types

Fix #131

@kkharji kkharji force-pushed the feat/support_sqlite_open_v2 branch 4 times, most recently from 6102385 to 4010351 Compare December 28, 2021 17:59
Conni2461
Conni2461 reviewed Dec 28, 2021
View reviewed changes
Conni2461
Conni2461 reviewed Dec 28, 2021
View reviewed changes
@kkharji kkharji force-pushed the feat/support_sqlite_open_v2 branch 5 times, most recently from a07f2e1 to 83a3428 Compare December 28, 2021 19:47
@kkharji
Copy link
Owner Author

kkharji commented Dec 28, 2021

okay that just weird. on my machine field types comes out uppercased but here lowercased.

@kkharji kkharji mentioned this pull request Dec 28, 2021
Closed
@dhruvmanila
Copy link
Collaborator

Hey @tami5, is there anything blocking this? Do you need any help? 😄

@kkharji
Copy link
Owner Author

kkharji commented Mar 3, 2022 •
edited
Loading

Hey @dhruvmanila, thanks for asking, well some tests started occasionally failing. unpredictable behavior of sort, see https://github.com/kkharji/sqlite.lua/runs/4653063219?check_suite_focus=true. So I'm kinda worried that merging this would cause unwanted side effect.

We can be lazy and just check if opts.open_mode is provide and thus use open_v2. This would make the PR mergable without fear of some unwanted side effects .

Other than that the issue with docgen.

I'm quite busy at the moment feel free to take over, thank you

@dhruvmanila
Copy link
Collaborator

Alright, I might look into it during the weekend, can't guarantee any fixes 😅

kkharji added a commit that referenced this pull request Mar 3, 2022
@kkharji
✅ field types are always uppercase
9ccd2a6 
At first I thought this issue introduced in
#132 but it's most likely
something todo with sqlite3 core changes.
@kkharji @Conni2461
feat: sqlite_open_v2 (take 2)
807ba86 
cleanup after 9ccd2a6

Co-authored-By: Simon Hauser <[email protected]>
@kkharji kkharji force-pushed the feat/support_sqlite_open_v2 branch from add5d55 to 807ba86 Compare March 3, 2022 18:39
@bliiir
Copy link

bliiir commented Jul 23, 2022

Hi guys. I'd like to express my interest in this getting merged as I would enble the use of @dhruvmanila's telescope-bookmarks plugin with Firefox. Thanks for all the hard work you have done and let me know if there is anything I can do to help

@kkharji kkharji merged commit 56c5aac into master Jul 23, 2022
@kkharji kkharji deleted the feat/support_sqlite_open_v2 branch July 23, 2022 21:01
@kkharji
Copy link
Owner Author

kkharji commented Jul 23, 2022 •
edited
Loading

@bliiir the pr was blocked for already resolved issue, I just totally forgot about it. Thanks for the gesture, I appreciate it

@bliiir
Copy link

bliiir commented Jul 24, 2022

Awesome. Works great - thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Conni2461 Conni2461 Conni2461 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

How to open database with immutable flag?
4 participants
@kkharji @dhruvmanila @bliiir @Conni2461