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

Fix issue where prepared statement can only be used once #61

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

mjmasn
Copy link
Contributor

@mjmasn mjmasn commented Mar 5, 2024

Move sqlite3_reset call after sqlite3_step in opsqlite_execute_prepared_statement

Fixes issue where new data does not correctly bind to a prepared statement after the first execution when inserting records, causing an unique constraint exception:

Error: Exception in HostFunction: [op-sqlite] SQLite code: 19 execution error: UNIQUE constraint failed: mytable.id

Can't share an exact reproduction but the general gist of the issue is doing something like this fails (id is a text primary key):

const statement = db.prepareStatement('INSERT INTO mytable (id, name) VALUES (?, ?)');

for (const record of records) {
  statement.bind([record.id, record.name]);
  statement.execute();
}

PS Happy to add some tests for this if it looks like a reasonable approach.

Fixes issue where new data does not correctly bind to a prepared statement after the first execution when inserting records, causing an unique constraint exception:

Error: Exception in HostFunction: [op-sqlite] SQLite code: 19 execution error: UNIQUE constraint failed: mytable.id
@ospfranco
Copy link
Contributor

I wonder why CI is not working. Did you merge main from your fork?

@ospfranco
Copy link
Contributor

ospfranco commented Mar 6, 2024

This is weird, if a statement has finished executing with previous params, rebinding it should have the opposite effect of this PR. Please do add the tests, while I read up on reset.

There is already a test for rebinding the prepared statement and it is working correctly?

it('allows re-binding a prepared statement', async () => {
      const statement = db.prepareStatement('SELECT * FROM User WHERE id=?;');
      statement.bind([1]);

      let results = statement.execute();
      expect(results.rows!._array[0].name === 'Oscar');

      statement.bind([2]);
      results = statement.execute();
      expect(results.rows!._array[0].name === 'Pablo');
    });

@ospfranco
Copy link
Contributor

Oh wow, nvm, it indeed fails. Weird!

Simulator Screenshot - iPhone 15 - 2024-03-06 at 16 07 43

@ospfranco
Copy link
Contributor

Moving it down indeed fixes the issue, though I still do not understand why. Will merge it for now. Thanks for the PR!

@ospfranco ospfranco merged commit f86d4a0 into OP-Engineering:main Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants