-
Notifications
You must be signed in to change notification settings - Fork 884
JAVA-1174: CAS insert with Mapper class #626
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
Conversation
|
Hi @mgurevin , thank you for your contribution! I agree that it would be a great idea for the Mapper to support My concern is that the current Another thought is that if you are using LWTs on a table that it may be best to always use LWT operations when updating rows in that table. Perhaps there is something we could configure at the |
|
Would greatly appreciate if something like this is implemented. For both |
|
Changed base branch since we plan to rename 3.0 to 3.x. |
| for (Mapper.Option opt : options) { | ||
| opt.checkValidFor(QueryType.SAVE, manager); | ||
|
|
||
| if (shouldIfNotExists(opt)) { |
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.
Maybe we should change the signature of appendTo to take Insert/Delete in parameter instead of Insert.Options/Delete.Options, so that you don't have to do a special case here and can apply ifNotExists() in appendTo.
|
Closing in favor of #826. Sorry for opening a separate PR, but I wanted to rebase on 3.x and clean up the option classes before, it was simpler that way. I've cherry-picked your commit as 621b22a, still properly attributed to you. @koscejev unfortunately, it's not possible to implement IF EXIST. As Andy explained, the mapper uses INSERT for both new and existing objects, and INSERT does not support IF EXIST. That's a limitation of the current implementation, the only workaround is to use an accessor method. |
| if (opt.isIncludedInQuery()) | ||
| this.optionTypes.add(opt.type); | ||
| } | ||
| this.options = options.values(); |
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.
For the record, we want to keep testing isIncludedInQuery(), to avoid storing extra prepared statement when unnecessary. For example, ttl(100) and ttl(2000) both generate the same query containing "TTL (?)", so it's not efficient to reprepare the query for each one.
No description provided.