Skip to content

feat: Support MySQL FOR SHARE locking syntax. Fixes: #10134. #12964

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Support MySQL FOR SHARE locking syntax. Fixes: #10134. #12964

wants to merge 1 commit into from
+57 −5

Conversation

@JetDrag
Copy link

@JetDrag JetDrag commented Nov 4, 2025 •
edited
Loading

Fixes: #10134.

- make select(...).with_for_update(read=True) emit FOR SHARE on MySQL 8+ (still LOCK IN SHARE MODE on older versions)
- allow combining FOR SHARE with OF/NOWAIT/SKIP LOCKED when dialect.supports_for_share is true
- extend mysql FOR UPDATE compile tests to cover shared-lock variants

Usage snapshots:

select(User).with_for_update(read=True)                    → FOR SHARE (MySQL 8+) / LOCK IN SHARE MODE (pre-8)
select(User).with_for_update(read=True, nowait=True, of=User.__table__) → FOR SHARE OF users NOWAIT (MySQL 8+)

CaselIT
CaselIT reviewed Nov 4, 2025
View reviewed changes
Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

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

thanks, it looks ok.

could you also add a changelog file here https://github.com/sqlalchemy/sqlalchemy/tree/main/doc/build/changelog/unreleased_20 with summary of the change, similar to the other ones that are there, like https://github.com/sqlalchemy/sqlalchemy/blob/main/doc/build/changelog/unreleased_20/6511.rst?

thanks!

@zzzeek zzzeek requested a review from sqla-tester November 4, 2025 23:59
sqla-tester
sqla-tester reviewed Nov 4, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

@zzzeek
Copy link
Member

zzzeek commented Nov 5, 2025

we've just moved to a new gerrit server and im using your PR to debug something, pardon the noise

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 2c6150b of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

sqla-tester commented Nov 5, 2025

New Gerrit review created for change 2c6150b: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6401

@JetDrag JetDrag closed this Nov 5, 2025
@JetDrag JetDrag reopened this Nov 5, 2025
@zzzeek
Copy link
Member

zzzeek commented Nov 5, 2025

hi just testing a comment here, ignore this

@sqla-tester
Copy link
Collaborator

sqla-tester commented Nov 5, 2025

Michael Bayer (zzzeek) wrote:

also testing a comment here

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6401

@zzzeek zzzeek requested a review from sqla-tester November 5, 2025 03:46
sqla-tester
sqla-tester reviewed Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 03d5e37 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

sqla-tester commented Nov 5, 2025

Patchset 03d5e37 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6401

@JetDrag
Copy link
Author

JetDrag commented Nov 5, 2025

thanks, it looks ok.

could you also add a changelog file here https://github.com/sqlalchemy/sqlalchemy/tree/main/doc/build/changelog/unreleased_20 with summary of the change, similar to the other ones that are there, like https://github.com/sqlalchemy/sqlalchemy/blob/main/doc/build/changelog/unreleased_20/6511.rst?

thanks!

Hi, @CaselIT, I've add the required changelog file 12964.rst. Review again plz.

@CaselIT
Copy link
Member

CaselIT commented Nov 6, 2025

thanks, I'll look at it soon

@sqla-tester
Copy link
Collaborator

sqla-tester commented Nov 6, 2025

Michael Bayer (zzzeek) wrote:

recheck

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6401

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

Reviewers

@sqla-tester sqla-tester sqla-tester left review comments

@CaselIT CaselIT CaselIT left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for mysql syntax "for share skip locked"

4 participants

@JetDrag @zzzeek @sqla-tester @CaselIT