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

.forShare() should not require a transaction #1742

Closed
txase opened this issue Oct 13, 2016 · 3 comments
Closed

.forShare() should not require a transaction #1742

txase opened this issue Oct 13, 2016 · 3 comments

Comments

@txase
Copy link

txase commented Oct 13, 2016

When adding .forShare() to a .select() query, the query will not return until any external pending transactions on the data are resolved. This does not require the select query to be made as part of a transaction. For example, in mysql the following query is valid and useful outside of a transaction:

SELECT id FROM accounts LOCK IN SHARE MODE

This will return only once all returned rows are free of any external transactions.

In the meantime, knex requires the following workaround of wrapping the simple select query in an unnecessary transaction:

knex.transaction((trx) => {
  return trx 
    .select('id')
    .from('accounts')
    .forShare()
})
@elhigu
Copy link
Member

elhigu commented Oct 14, 2016

In mysql what does

SELECT id FROM accounts LOCK IN SHARE MODE

without transaction do exactly? I skimmed through http://dev.mysql.com/doc/refman/5.7/en/innodb-locking-reads.html but didn't see any mention of using it outside of transaction.

I suppose mysql could create implicit transaction where it runs that select and frees lock right after or it may lock selected rows until connection is closed...

I'm pretty sure that this problem has not anything to do with knex, since as far as I know knex doesn't do any special waiting for selects with lock statements.

Could you provide complete test code demonstrating the problem you got?

@txase
Copy link
Author

txase commented Oct 14, 2016

I tested it by opening two mysql cli connections to the same database. In the first connection I did something like:

BEGIN;
UPDATE accounts SET status = 'open' WHERE id = 58;

(Note that I did not commit the transaction yet)

In the second connection I did:

SELECT * FROM accounts WHERE id = 58 LOCK IN SHARE MODE;

The second connection did not return until the first connection's transaction was committed.

This is a potential issue with knex because knex won't add the 'LOCK IN SHARE MODE' part to the query if .forShare() is not called within a transaction: https://github.com/tgriesser/knex/blob/67b62d3213aa6ade68bdf84cb95b3410a1fd8053/src/query/compiler.js#L400-L408.

@wubzz
Copy link
Member

wubzz commented Feb 26, 2018

Fixed by #2475

@wubzz wubzz closed this as completed Feb 26, 2018
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

No branches or pull requests

3 participants