Skip to content

Conversation

@nechutny
Copy link
Contributor

@nechutny 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 master branch 2 times, most recently from cf35cfb to 5c78161 Compare July 6, 2016 07:19
@nechutny
Copy link
Contributor Author

ping @dg

$query = "UPDATE {$this->delimitedTable} SET ?set" . $this->tryDelimite($this->buildConditions());
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)) {
Copy link
Member

Choose a reason for hiding this comment

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

missing space after if

@nechutny
Copy link
Contributor Author

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

@dg
Copy link
Member

dg commented Aug 18, 2016

Thanks!

@dg
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
Copy link
Contributor Author

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.

@dg dg merged commit b816407 into nette:master Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants