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 support for limit in update and delete queries for MySQL driver #140

Merged
merged 1 commit into from Aug 18, 2016

Conversation

@nechutny
Copy link
Contributor

nechutny commented Jul 6, 2016

Hi,

I've prepared this pull request as solution for issue #120. It add support for limit in update and delete queries in MySQL as desribed in documentation http://dev.mysql.com/doc/refman/5.7/en/update.html + http://dev.mysql.com/doc/refman/5.7/en/delete.html.

It add new constants to ISupplementalDriver SUPPORT_UPDATE_LIMIT and SUPPORT_DELETE_LIMIT so each driver can define support. If it's not supported, then is throw same exception as before.

@nechutny nechutny force-pushed the nechutny:master branch 2 times, most recently from cf35cfb to 5c78161 Jul 6, 2016
@nechutny

This comment has been minimized.

Copy link
Contributor Author

nechutny commented Aug 16, 2016

ping @dg

if ($this->limit !== NULL || $this->offset) {
throw new Nette\NotSupportedException('LIMIT clause is not supported in UPDATE query.');
if($this->driver->isSupported(ISupplementalDriver::SUPPORT_UPDATE_LIMIT)) {

This comment has been minimized.

Copy link
@dg

dg Aug 16, 2016

Member

missing space after if

if ($this->limit !== NULL || $this->offset) {
throw new Nette\NotSupportedException('LIMIT clause is not supported in DELETE query.');
if($this->driver->isSupported(ISupplementalDriver::SUPPORT_DELETE_LIMIT)) {

This comment has been minimized.

Copy link
@dg

dg Aug 16, 2016

Member

missing space after if

@nechutny nechutny force-pushed the nechutny:master branch from 5c78161 to 8bf1635 Aug 18, 2016
@nechutny

This comment has been minimized.

Copy link
Contributor Author

nechutny commented Aug 18, 2016

Missing space after 'if' added and squashed into single commit.

@dg

This comment has been minimized.

Copy link
Member

dg commented Aug 18, 2016

Thanks!

@dg

This comment has been minimized.

Copy link
Member

dg commented Aug 18, 2016

Now I think about it: what about apply limit & offset always, without checking of SUPPORT_UPDATE_LIMIT? If it will not be supported by database, it throws exception, right?

@nechutny

This comment has been minimized.

Copy link
Contributor Author

nechutny commented Aug 18, 2016

I've searched for LIMIT clause support in databases supported by Nette\Database and only Sqlite has compile switch to enable/disable limit in update/delete. So remove check and leave it on database driver to throw exception seems as better solution. I'll update commit.

@nechutny nechutny force-pushed the nechutny:master branch from 8bf1635 to bc77de5 Aug 18, 2016
@nechutny nechutny force-pushed the nechutny:master branch from bc77de5 to 913d3d4 Aug 18, 2016
@dg dg merged commit b816407 into nette:master Aug 18, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 86.457%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.