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

Add row-level locking options #1810

Merged
merged 3 commits into from Apr 12, 2018
Merged

Add row-level locking options #1810

merged 3 commits into from Apr 12, 2018

Conversation

@ricardograca
Copy link
Member

@ricardograca ricardograca commented Apr 11, 2018

  • Related Issues: #324
  • Previous PRs: #516

Introduction

Follow-up to the previous implementation on the linked PR.

Motivation and Proposed solution

This adds tests and uses a slightly different implementation that is less repetitive and more easily expandable if needed.

Closes #516.

Current PR Issues

Tests aren't very exhaustive.

There's also no check to see if the lock options are being applied to a select statement, but it's possible that Knex already takes care of this. In any case this is an advanced feature, so users that want it should know what they're doing.

- This only works inside a transaction and seems to make no effect on
MySQL where it will always lock even without the option.

- There's also no support for this feature on SQLite.
@ricardograca ricardograca added this to To Do in Version 0.14.0 via automation Apr 11, 2018
@ricardograca ricardograca moved this from To Do to In progress in Version 0.14.0 Apr 12, 2018
@ricardograca ricardograca merged commit b4f77ae into master Apr 12, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Version 0.14.0 automation moved this from In progress to Done Apr 12, 2018
@ricardograca ricardograca deleted the rg-transaction-locking branch Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant