Conversation
vttablet cannot support those two functions in upserts. Marking them as keywords automatically prevents their use. This way, we don't have to explicitly look for these keywords inside upserts to prevent them. If/when we decide to support them, we can add these keywords to the keyword_as_func rule in the parser.
Upsert is mostly feature complete. There are still two issues to resolve: 1. RowsAffected is returned as 0 if no rows were modified. So, we cannot assume that no rows were matched. So, the value cannot be used to verify if the dup key matched a pk or a unique key. 2. I mistooke VALUES to be VALUE, and coded defense against using that construct in the parser. It turns out that the parser already allows use of VALUES. So, I'll have to find a different way to prevent that usage for upserts.
Found a good way to know which index caused a dup key error. The error message itself contains that info. This is better than looking at RowsAffected because there are other conditions that cause that field to be 0.
This is a more correct fix for disallowing values function. I've verified that this function is only supported for the upsert construct: https://dev.mysql.com/doc/refman/5.5/en/miscellaneous-functions.html#function_values. So, it's safe to disallow this at the parser level, just as we do for last_insert_id.
go/vt/tabletserver/query_executor.go
Outdated
Contributor
There was a problem hiding this comment.
So here we get a duplicate key error and decide to ignore the insert error and try an update instead. I think it may worth leaving a comment.
Contributor
Author
There was a problem hiding this comment.
I'll add this in a future PR.
Contributor
|
LGTM. One minor comment. |
Contributor
|
FYI, I see someone else merges change before this PR and you may need to resolve conflict locally before merging. |
sougou
added a commit
that referenced
this pull request
Aug 18, 2015
tabletserver: Add support for upsert
notfelineit
pushed a commit
to planetscale/vitess
that referenced
this pull request
Sep 21, 2022
vitessio#1000) * Address additional causes of OnlineDDL test flakiness This work makes the following changes: - Replaces any time.Sleep calls with a call to wait for the appropriate status(es) - Use WaitForStatus:Failed,Cancelled for cancel tests - Use CheckForStatus:Failed,Cancelled for cancel tests (instead of just Cancelled) - As we see both in the CI but we were requiring cancelled - See: vitessio#11011 Signed-off-by: Matt Lord <mattalord@gmail.com> * Always print status from the wait Signed-off-by: Matt Lord <mattalord@gmail.com> * Require Cancelled state for for Cancel issued by user Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove unnecessary change * return error if unable to set 'cancelled_timestamp' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a partial support for upsert. The feature covers the most important use case: Single row insert where the dup key is due to the primary key index.
I've also explicitly disabled usage of the VALUES and LAST_INSERT_ID functions. Support for those can be added if we decide to extend this feature to allow multiple rows, where these functions become more relevant.