-
Notifications
You must be signed in to change notification settings - Fork 52
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
Missing limit in mysql update queries #65
Comments
This was actually an intentional decision, because I couldn't think of a single use case for having an update statement with |
I use it for performance reasons, when the amount of rows to be updated are known. For example: UPDATE users SET disabled = 1 WHERE id IN (2, 3, 4) LIMIT 3; |
Does that actually change performance? I assume that |
Yes, that does not affect to performance. I don't know why, I always used limit in these queries. 😄 Anyway, I guess that this library is focused only in build sql queries using the correct syntax for each database engine. So, if a engine allows to use these clauses, why have these limits? For example, there's no way to update a row directly based in other clauses than indexes. For example, to update the 5 users with more likes: UPDATE users SET trending = 1 ORDER BY likes DESC LIMIT 5; Without order and limit, it forces to select the users first, get their ids and then execute the update. There's no orderBy in the delete query, so it makes hard to delete, for example, the 100 oldest elements in a table: DELETE items ORDER BY lastActivity LIMIT 100; |
The standard SQL way to do this is with a sub-select: UPDATE users SET trending = 1 WHERE id IN (
SELECT id FROM users ORDER BY likes DESC LIMIT 5
) Latitude supports this syntax using |
I'd be happy to take a PR to add |
Ok, I'm not sql expert so I'll try to do not use these clauses for now. |
For deletes best practice is to always use a limit to prevent coding errors from deleting the entire table. |
I'm getting this error:
and have noticed that limit capability is missing in Update queries.
orderBy is missing too.
The text was updated successfully, but these errors were encountered: