-
-
Notifications
You must be signed in to change notification settings - Fork 263
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: DB2 supports select .. for update queries #3315
Conversation
TestQueryFindPagedList.test_forUpdate with db2 is failing, because of: error code: https://www.sqlerror.de/db2_sql_error_-511_sqlstate_42829.html (you can only use for update if the select clause uses one table)
|
@Override | ||
protected String withForUpdate(String sql, Query.LockWait lockWait, Query.LockType lockType) { | ||
// NOWAIT and SKIP LOCKED not supported with Db2 | ||
return sql + " for update"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have another question about this PR.
The test TestQueryFindPagedList.test_forUpdate throws an exception because a query with join cannot be executed.
"For update" cannot be executed in these constellations: https://www.sqlerror.de/db2_sql_error_-511_sqlstate_42829.html
Would it be a viable way to check the SQL to see if any of the "forbidden" keywords appear in the command and then decide whether for update is possible? (e.g.: join, distinct, order by, group by, etc.) Or does it work if ebean passes on the exception to the DB (like the existing solution)?
Something like this:
protected String withForUpdate(String sql, Query.LockWait lockWait, Query.LockType lockType) {
// NOWAIT and SKIP LOCKED not supported with Db2
if (checkKeywords(sql)) {
return sql + " for update";
} else {
return super.withForUpdate(sql, lockWait, lockType);
}
}
private boolean checkKeywords(String sql) { ... }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viable way to check the SQL to see if any of the "forbidden" keywords appear in the command and then decide whether for update is possible?
I think that it is better to throw the exception and let the develop decide how they want to deal with the limitations of the database (it's not just DB2 with limitations here). There is io.ebean.Database.platform()
to allow developers to determine the current platform and potentially perform platform specific logic if required. I realise this can be a bit of a pain so I instead expect developers to use the "lowest common working functionality" across the databases being used instead.
So yes I'm happy with the exception being thrown, its nice and simple and honest to the developer.
Do you have some scenario where you think it would be better to do a keyword check? If you think there is a better approach then I'm happy to hear about it and see where it leads us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback, we don't have a usecase that forces to check the keywords. We are also agree with the merged solution.
Hello @rob-bygrave ,
I've implemented the select - for update structure for DB2.
IBM documentation of select - for update: https://www.ibm.com/docs/en/db2/11.5?topic=statement-update-clause
nowait & skip locked are only for update queries: https://www.ibm.com/docs/en/db2/11.5?topic=statements-update#sdx-synid_nowait
Can you please take a look at it?
Kind regards
Noemi