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.

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

[Request]: Bring back sync execute #173

Open
pranshuchittora opened this issue Oct 25, 2024 · 2 comments
Open

[Request]: Bring back sync execute #173

pranshuchittora opened this issue Oct 25, 2024 · 2 comments

Comments

@pranshuchittora
Copy link

pranshuchittora commented Oct 25, 2024 •
edited
Loading

What do you need?

Hey, I am using op-sqlite and in the newest version the execute api was changed from sync to async.
My use-case requires sync query execution in some cases which can't be changed to async.

1dbc828#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R127

Is there any possibility to bring back the sync execute? or any workaround for the same?

Thanks
@Volland
Copy link
Contributor

Volland commented Oct 29, 2024

we had a similar request some of our api is sync only

@ospfranco
Copy link
Contributor

@rflopezm

Here was the change:

https://github.com/OP-Engineering/op-sqlite/pull/142/files#diff-cd0866c167878907e99d1e94024bc05e6f673d0e5228bf11448932f2aaa4d0c2L320

Now there are a couple of differences, before you go re-implementing this interface:

  • Don't use SmartHostObject or execute_with_host_objects, the abstraction will cost too much on runtime, use the same as current execute but just don't execute it in a thread.
  • Create a test (or maybe some tests cases) on queries.spec.ts. Make sure it returns the correct data
  • Also create the same function for libsql. The tests already run automatically here, so as soon as you have created the test on queries.spec.ts it will let you know if something is missing.
  • No need to modify the transactions for now. It's too much work to keep both interfaces working within the async context and now it might mess up with reactive queries. Bringing a simple .execute should be enough.
  • If you need more info on interacting with the JSI there are guides and tutorials on my website

rikur added a commit to rikur/op-sqlite that referenced this issue Nov 19, 2024
@rikur
Add synchronous execute function
b00d272 
Fixes OP-Engineering#173

Add synchronous execute API to the project.

* **cpp/DBHostObject.cpp**
  - Add a new `executeSync` function for synchronous execution.
  - Directly call `opsqlite_execute` without using `invokeAsync`.
  - Register the new `executeSync` function in the function map.
* **cpp/libsql/bridge.cpp**
  - Add a new `executeSync` function for synchronous execution.
  - Directly call `libsql_query_stmt` without using `libsql_prepare`.
* **cpp/DBHostObject.h**
  - Declare the new `executeSync` function.
* **cpp/libsql/bridge.h**
  - Declare the new `executeSync` function.
* **example/src/tests/queries.spec.ts**
  - Add test cases for the new `executeSync` function.
* **README.md**
  - Update documentation to include the new `executeSync` function.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/OP-Engineering/op-sqlite/issues/173?shareId=XXXX-XXXX-XXXX-XXXX).
@rikur rikur mentioned this issue Nov 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add synchronous execute function rikur/op-sqlite
3 participants
@Volland @ospfranco @pranshuchittora