Skip to content

feat: implement "FOR UPDATE OF" for postgres driver #7040

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.

Sign up for GitHub

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

Jump to bottom
Merged
AlexMesser merged 2 commits into from
Feb 9, 2021
Merged

feat: implement "FOR UPDATE OF" for postgres driver #7040

AlexMesser merged 2 commits into from
Feb 9, 2021

Conversation

@ejose19
Copy link
Contributor

@ejose19 ejose19 commented Nov 8, 2020

Description of change

Fixes #4084

This PR implements some parts of the unmerged #5253 PR, improving it to allow specifying multiple tables and checking whether the referenced tables exists in the query.

Also, there are some improvements to related methods JSDocs/Type definitions, removing invalid where space, and adding missing lock mode to FindOptions.

Note for whoever want to implement this for other drivers

  • Oracle uses a different syntax, specifying table.column or column instead of table after the OF clause, so an special treatment needs to be done when oracle is the driver. source1 source2

  • MySQL / Aurora do indeed support this syntax in their latest versions, but tests are using an outdated version of them, so a new updated image is needed for properly testing the feature. source

  • MySQL / Aurora in their latest version does not need to use a different if for return " LOCK IN SHARE MODE";, as they support the FOR SHARE syntax. *same source as above

  • MySQL / Aurora / Oracle supports SKIP LOCKED and NOWAIT, so when new tests images are added they can be included in the corresponding if. *same sources as above

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@ejose19 ejose19 mentioned this pull request Nov 10, 2020
Closed
@AlexMesser AlexMesser merged commit fde9f07 into typeorm:master Feb 9, 2021
@AlexMesser
Copy link
Collaborator

AlexMesser commented Feb 9, 2021

thank you for contribution!

herenickname
herenickname reviewed Apr 22, 2021
View reviewed changes
src/query-builder/SelectQueryBuilder.ts
* Sets locking mode.
*/
setLock(lockMode: "optimistic", lockVersion: Date): this;
setLock(lockMode: "pessimistic_read"|"pessimistic_write"|"dirty_read"|"pessimistic_partial_write"|"pessimistic_write_or_fail"|"for_no_key_update", lockVersion?: undefined, lockTables?: string[]): this;
Copy link

@herenickname herenickname Apr 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a version of setLock without lockVersion param?

@ejose19
Copy link
Contributor Author

ejose19 commented Apr 22, 2021

You should use typescript, null is not a valid value to be passed as 2nd argument. It gives an error: Argument of type 'null' is not assignable to parameter of type 'undefined'.ts(2345)

The way the original setLock is designed, you need to pass undefined as 2nd argument if you want to use lockTables, changing it would be a breaking change and one that wouldn't have been merged into this major release of typeorm.

@herenickname
Copy link

herenickname commented Apr 22, 2021

You should use typescript, null is not a valid value to be passed as 2nd argument. It gives an error: Argument of type 'null' is not assignable to parameter of type 'undefined'.ts(2345)

The way the original setLock is designed, you need to pass undefined as 2nd argument if you want to use lockTables, changing it would be a breaking change and one that wouldn't have been merged into this major release of typeorm.

I have tried undefined before null. Anyway, I found a bug, it's on my side, thanks for this feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@herenickname herenickname herenickname left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

QueryBuilder with leftJoinAndSelect and pessimistic_write lock (postgres)

3 participants

@ejose19 @AlexMesser @herenickname