react-native-nitro-sqlite icon indicating copy to clipboard operation
react-native-nitro-sqlite copied to clipboard

Cannot execute multiple statements with one call

Open vhakulinen opened this issue 2 years ago • 11 comments

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:

https://github.com/margelo/react-native-quick-sqlite/blob/d70108549e7d5855a497dc14be25e7dafb00524c/cpp/sqliteBridge.cpp#L260

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

vhakulinen avatar Apr 13 '23 12:04 vhakulinen

Are you getting this using batch operations

pbbadenhorst avatar Apr 17 '23 15:04 pbbadenhorst

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

vhakulinen avatar Apr 18 '23 05:04 vhakulinen

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.

ospfranco avatar Nov 09 '23 13:11 ospfranco

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.

vhakulinen avatar Nov 09 '23 15:11 vhakulinen

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

ospfranco avatar Nov 09 '23 15:11 ospfranco

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.

vhakulinen avatar Nov 09 '23 17:11 vhakulinen

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

ospfranco avatar Nov 09 '23 17:11 ospfranco

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.

vhakulinen avatar Nov 09 '23 17:11 vhakulinen

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

ospfranco avatar Nov 09 '23 17:11 ospfranco

Why not just improve this library?

vhakulinen avatar Nov 09 '23 17:11 vhakulinen

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.

ospfranco avatar Nov 09 '23 18:11 ospfranco