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

Cannot execute multiple statements with one call #3

Open
vhakulinen opened this issue Apr 13, 2023 · 11 comments
Open

Cannot execute multiple statements with one call #3

vhakulinen opened this issue Apr 13, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@vhakulinen
Copy link

Consider the following sql script:

CREATE TABLE foo (id INTEGER);
CREATE TABLE bar (id INTEGER);

With the current interface, only the first statement will be executed and no error is given. I would've expected to have the whole sql script executed, or at least to get an error.

I digged through the code, and found out that the pzTail parameter is unused in the sqliteExecute's prepare call:

int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &statement, NULL);

Apparently that pointer gets populated with the remaining text that is yet to be processed: https://sqlite.org/forum/info/2a9775b88a90d2e6.

@pbbadenhorst
Copy link

Are you getting this using batch operations

@vhakulinen
Copy link
Author

I haven't tried it but looking at the source code, its not using the pzTail param either.

@Montchy Montchy added the bug Something isn't working label Aug 14, 2023
@ospfranco
Copy link

Interesting, but you are better off using the executeBatch API, that one will not only execute all your operations but also wrap them in a transaction.

@vhakulinen
Copy link
Author

Lol, no. Splitting a code injected SQL migration on runtime with some special comments is error prone (which we're doing at the moment) and PITA especially when they are meant to be run in one go either way.

Using transactions should be left to the API consumer to do.

@ospfranco
Copy link

ah, how could I've known? You are always free to create a PR

@vhakulinen
Copy link
Author

vhakulinen commented Nov 9, 2023

My apologies if the tone of my message was inappropriate.

Usually (or at least based on my experience) database libraries/drivers execute all the provided statements, or documents or errors out in cases when they don't. Its really unexpected behavior to silently execute input only partially.

@ospfranco
Copy link

well... I partially implemented this, the problem is that I'm not sure it is really compatible with the current API? How would you pass different params for each row of the string? I would also have to modify the metadata object and the result object to return an array instead of a single object, which would increase the complexity a bit.

In any case, here is a partial implementation, it consumes all the statements but the returned data is not correct at all.

https://github.com/OP-Engineering/op-sqlite/tree/consume-entire-statement

@vhakulinen
Copy link
Author

The API needs to be split between

  1. execute only
  2. execute and return data

If that doesn't fit in the architecture, throw error instead of failing silently.

@ospfranco
Copy link

alright, as you wish. It's implemented in op-sqlite.

@vhakulinen
Copy link
Author

Why not just improve this library?

@ospfranco
Copy link

This one now belongs to Margelo. I started hacking around on the new one out of curiosity and decided to publish it as a new one as it helps me get customers. Nothing against them, just trying to get something back for my time and effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants