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 ability to SELECT… FOR UPDATE to ActiveRecord #11730

Open
wartur opened this Issue Jun 9, 2016 · 23 comments

Comments

Projects
None yet
9 participants
@wartur
Copy link

commented Jun 9, 2016

Очень не хватает поддержки исключительной блокировки в билдере запросов FOR UPDATE или поддержки lock in share mode (без переключения в более строгий уровень транзакции)

Если lock in share mode это не обязательно, достаточно использовать SERIALIZABLE, то с FOR UPDATE все сложнее, его так просто не заюзаешь. Почему бы это не добавить?

Её можно реализовать вот так.

            $execOrder = ExecOrder::find()
                    ->forUpdate()
//    или        ->lockInShare() // если нужно, но она не нужна в общем случае
                    ->where(['status' => ExecOrder::STATUS_WAIT])
                    ->orderBy(['id' => SORT_ASC])->one();

Да есть кое-какая разница между базами данных в синтаксисе FOR UPDATE, но я думаю это нужная штука.

Эта функциональность нужна где-то здесь
https://github.com/yiisoft/yii2/blob/master/framework/db/QueryBuilder.php#L88
Ну или видимо она должна быть некой абстрактной и поддержана разными БД по разному. Как я слышал от товарищей синтаксис lock in share mode / FOR UPDATE (MySQL) отличается от postgresql и оракла.

Почему это стоит добавить?
Потому что, если использовать транзакции невозможно получить исключительную блокировку удобными способами.

Альтернатива сейчас?
Использовать findBySql, но это не спортивно!

Q A
Yii version 2.0.8
PHP version 5.6.22
Operating system Windows 7 x64

@samdark samdark changed the title Предлагаю добавить SELECT… FOR UPDATE в ActiveRecords Add ability to SELECT… FOR UPDATE to ActiveRecord Jun 9, 2016

@samdark

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@samdark

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

Нужно исследование на тему поддержки этого дела каждой из баз, которые умеет Yii.
Если будет поддерживаться большинством, можно запилить.

@AnikanovD

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2016

Oracle, MySQL, PostgreSQL, have identical syntax of FOR UPDATE.
Differences happen when uses LOCK IN SHARE MODE | FOR SHARE.

http://docs.oracle.com/database/121/SQLRF/statements_10002.htm#i2126016
https://www.postgresql.org/docs/9.0/static/sql-select.html#SQL-FOR-UPDATE-SHARE
http://dev.mysql.com/doc/refman/5.6/en/innodb-locking-reads.html

MSSQL not sportive :(

FOR UPDATE is not valid SQL Server syntax in a regular SQL statment, it is used when you create a cursor.
http://dba.stackexchange.com/questions/35532/difference-between-updlock-and-for-update

@jetexe

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2017

Is this enchartment planned to be released?

@samdark

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

@jetexe if someone will implement that for all DBs we support then yes.

@jetexe

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2017

Как уже писали выше, под MSSQL либо ставить заплатку (что не правильно и подойдет только для части проектов), либо нужен специалист по этой платформе, которого в мире PHP найти проблематично

@samdark

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

@sergeymakinen what do you think?

@skynin

This comment has been minimized.

Copy link

commented Jul 20, 2017

Мне эта вещь нужна будет скоро в проекте(в модуле "финансовых" расчетов), так что в любом случае буду патчить, для MySQL.
Поделиться будет не жалко, только тогда нужна бы консультация, указание - как корректнее, а не как мне вздумается, "абы работало".

@sergeymakinen

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

AFAIK it's definitely possible with (UPDLOCK), the problem is both SELECT and UPDATE must be executed in a transaction with SET TRANSACTION ISOLATION LEVEL SERIALIZABLE.
Because of this we can't (technically we can but it has too many drawbacks IMO) auto enclose commands in a transaction. But we can check for a transaction and throw an exception explaining this situation. I can take care of it.
If there's something wrong, please correct me.

Edit: forgot to mention - I'm talking about MSSQL. For other DBMSes it should not be a problem. A CUBRID case is special - I have no clue how to implement such locking in this DBMS.

@samdark

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

CUBRID is similar to MySQL. At least it should be.

@sergeymakinen

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

It looks like that it's supported (at least in 10.x).
So I will implement it. The only challenge I see is to write tests that will properly cover different locking cases so an implementation will be consistent across all DBMSes.

@samdark

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

Yes. Tests are tricky.

@IvoPereira

This comment has been minimized.

Copy link

commented Mar 6, 2018

Any news on this @samdark or you totally abandoned the idea?

@samdark

This comment has been minimized.

Copy link
Member

commented Mar 6, 2018

Idea is OK. Noone is taking care of it at the moment as far as I know. Do you want to?

@bupy7

This comment has been minimized.

Copy link

commented Jun 29, 2018

@skynin

This comment has been minimized.

Copy link

commented Jul 2, 2018

bupy7 спасибо. но я давно уже расширил стандартный ActiveQuery и забыл об этой проблеме :)

и пишу просто:
MyActiveRecord::find()->where(...)->forUpdate()->...
MyActiveRecord::find()->where(...)->forRead()->...
автокоплит в IDE тоже работает, поэтому оба метода начинаются с for

расширил так, что в использующем коде не нужно ссылаться на эти расширения.
то есть - одним махом можно их включить, или выключить, и переписывать ничего надо, только forUpdate() и forRead() конечно вызовут ошибку.

Не делал пул реквест, потому что наверняка забракуют, да и тесты не писал, там и не к чему их особо писать,
перекрыт init() в Connection, createQueryBuilder() в Schema, build() в QueryBuilder и трейт для ActiveQuery, или наследование

в конфиге настроек к бд - ставится этот Connection и все.

@bupy7

This comment has been minimized.

Copy link

commented Jul 2, 2018

@skynin Great! If you would make a pull-request it will be cool.

@skynin

This comment has been minimized.

Copy link

commented Jul 2, 2018

I think my pattern is no universal, for any DB, but maybe it will help other programmer.
I use this pattern in my projects.

https://github.com/skynin/mysql-activequery

@samdark

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

Yes, having it for all DBs is more complicated but I like how it's done.

@Jestergnet

This comment has been minimized.

Copy link

commented Sep 26, 2018

bupy7 спасибо. но я давно уже расширил стандартный ActiveQuery и забыл об этой проблеме :)

и пишу просто:
MyActiveRecord::find()->where(...)->forUpdate()->...
MyActiveRecord::find()->where(...)->forRead()->...
автокоплит в IDE тоже работает, поэтому оба метода начинаются с for

расширил так, что в использующем коде не нужно ссылаться на эти расширения.
то есть - одним махом можно их включить, или выключить, и переписывать ничего надо, только forUpdate() и forRead() конечно вызовут ошибку.

Не делал пул реквест, потому что наверняка забракуют, да и тесты не писал, там и не к чему их особо писать,
перекрыт init() в Connection, createQueryBuilder() в Schema, build() в QueryBuilder и трейт для ActiveQuery, или наследование

в конфиге настроек к бд - ставится этот Connection и все.

А более подробнее можете описать установку?

@skynin

This comment has been minimized.

Copy link

commented Sep 26, 2018

А более подробнее можете описать установку?

копируете github.com/skynin/mysql-activequery в свой проект.
правите неймспейсы, если нужно
остальное написано в #11730 (comment)

@Jestergnet

This comment has been minimized.

Copy link

commented Sep 26, 2018

А более подробнее можете описать установку?

копируете github.com/skynin/mysql-activequery в свой проект.
правите неймспейсы, если нужно
остальное написано в #11730 (comment)

Заменил namespace, раскидал к орилинальным файлам, изменил в конфиге и после добавления в конструкцию параметра forUpdate() пишет неизвестные метод ActiveRecord. Что не так?

@skynin

This comment has been minimized.

Copy link

commented Sep 26, 2018

Заменил namespace, раскидал к орилинальным файлам, изменил в конфиге и после добавления в конструкцию параметра forUpdate() пишет неизвестные метод ActiveRecord. Что не так?

я не описывал функционал фреймворка Yii2.
но ок.
Чтобы использовать YourActiveQuery - нужно переопределить метод find у вашей модели
Чтобы использовать эти два метода в своем кастомном ActiveQuery - нужно наследовать свой ActiveQuery от YourActiveQuery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.