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

sqlite error: no such table: tablename_returning_temp #107

Open
Sayuki0x opened this issue Jan 2, 2021 · 7 comments
Open

sqlite error: no such table: tablename_returning_temp #107

Sayuki0x opened this issue Jan 2, 2021 · 7 comments
Labels
type: bug Something isn't working as intended or expected.

Comments

@Sayuki0x
Copy link

Sayuki0x commented Jan 2, 2021

description

I attempted to port a class i had which used knex to manage an sqlite database to this library. I'm getting these type of errors from both the sql.js and the sqlite3 driver.

It seems to happen specifically if I perform several rapid calls to the model.create() function.

Along this line of thinking, I attempted to create a small reproduction repo, but i constructed some similar data to what i'm saving and hammered the create() function as fast as I could, and had no dice getting it to throw. You can reproduce this bug if you run yarn test on this repo on the trilogy branch:

https://github.com/vex-chat/libvex-js

I apologize I wasn't able to get a small reproduction of this, but hopefully you have some idea of what is going on by the stack trace.

error

    no such table: messages_returning_temp
      at c.handleError (node_modules/sql.js/dist/sql-wasm.js:89:254)
      at c.exec (node_modules/sql.js/dist/sql-wasm.js:87:6)
      at Object.runQuery (node_modules/trilogy/dist/helpers.js:118:37)
      at Model.create (node_modules/trilogy/dist/model.js:51:24)

expected

no error should be thrown

actual

error is thrown

Thank you

@Sayuki0x Sayuki0x added the type: bug Something isn't working as intended or expected. label Jan 2, 2021
@haltcase
Copy link
Owner

haltcase commented Jan 3, 2021

@ExtraHash thanks for the report and the branch that reproduces it, I'll look into it when I get a chance!

@Sayuki0x
Copy link
Author

Sayuki0x commented Jan 3, 2021

Sounds good, let me know if I can help by providing more info or anything.

@Sayuki0x
Copy link
Author

Sayuki0x commented Jan 9, 2021

Hello again, I happened to be reading the function i was having trouble with on this and had an epiphany about what was triggering it, and was able to build a small reproduction repo:

https://github.com/ExtraHash/trilogy_bug_reprod/blob/master/src/index.ts#L23

Apparently, it's Promise.all() that is the cause. If i load up an array full of model.create() promises, and then try to resolve them at once with Promise.all(), it throws:

(node:23343) UnhandledPromiseRejectionWarning: Error: no such table: posts_returning_temp
    at c.handleError (/home/ender/typescript-boilerplate/node_modules/sql.js/dist/sql-wasm.js:89:254)
    at c.exec (/home/ender/typescript-boilerplate/node_modules/sql.js/dist/sql-wasm.js:87:6)
    at Object.runQuery (/home/ender/typescript-boilerplate/node_modules/trilogy/dist/helpers.js:118:37)
    at async Model.create (/home/ender/typescript-boilerplate/node_modules/trilogy/dist/model.js:51:24)
    at async Promise.all (index 49)
    ```

@haltcase
Copy link
Owner

Seems like there's a race condition where that temp table is created/removed:

https://github.com/citycide/trilogy/blob/e4833a587651cfb9349087a9a715ee1d5908acb7/src/schema-helpers.ts#L131-L136

This is probably best solved along with #102, although interestingly I have used Promise.all with a bunch of create calls before without running into this. To be continued...

@haltcase
Copy link
Owner

haltcase commented Aug 8, 2021

Adding a method for bulk insert is still on the table, but it's worth noting this would also likely be resolved if we could rely on native returning behavior. SQLite finally got that feature in 3.35.0 which sql.js supports while the Node driver hasn't been updated to include it.

That would replace the workaround in trilogy that uses temporary separate tables and the triggers that pull from them.

We'll need this PR to be merged:
TryGhost/node-sqlite3#1476

@zshannon
Copy link

Running into this bug now, any advice on how to insert about 10 rows around the same time?

@haltcase
Copy link
Owner

haltcase commented Aug 12, 2022

I think at this point node-sqlite3, knex, and sql.js have all been updated to support SQLite's returning feature, which could be implemented here and might fix the issue. I'd definitely welcome any PRs seeking to address this, and as I mentioned in #115 would be happy to invite collaborators to the project who might be able to invest more time than I can at the moment.

Another option as a workaround would be to wait for the previous insert to finish before starting the new one, which I know isn't ideal.

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

No branches or pull requests

3 participants