-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fixing error on deletes from owning table for a lookup being populated by CreateLookupVindex after a SwitchWrites #8701
Fixing error on deletes from owning table for a lookup being populated by CreateLookupVindex after a SwitchWrites #8701
Conversation
Signed-off-by: Florent Poinsard <[email protected]>
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.
LGTM; also validated manually with some edgecases.
Does this also fix #7336? |
It does not fix the issue mentioned. |
…kup unique Signed-off-by: Florent Poinsard <[email protected]>
@harshit-gangal, as we talked about offline: I have modified the code to change the DML's opcode in the planbuilder instead of doing it in the engine |
…x-during-backfilling Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
…x-during-backfilling Signed-off-by: Florent Poinsard <[email protected]>
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.
we can remove the changes in the engine test as the changes are associated with the planner.
Also, add unit test at executor level.
Signed-off-by: Florent Poinsard <[email protected]>
…e executor level Signed-off-by: Florent Poinsard <[email protected]>
@harshit-gangal, thank you for the review! I have applied the recommended changes |
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.
changes look good.
More tests are needed in executor and planner where a higher cost Vindex is selected as the lower cost vindex was in backfill.
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
…dexes Signed-off-by: Florent Poinsard <[email protected]>
…ing selected Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
…x-during-backfilling Fixing error on deletes from owning table for a lookup being populated by CreateLookupVindex after a SwitchWrites
Description
After creating two keyspaces and a lookup Vindex using
CreateLookupVindex
, every delete query that uses the same lookup Vindex starts failing. Below are: a sample query using the columnc2
(which is our lookup Vindex); and the error that we got after executing the query.The reason behind this error is the following: the engine tries to execute a
delete equal
however, the lookup Vindex used to resolve the shard is inwrite_only
mode, meaning we cannot resolve to which shard this query needs to be routed. The default behavior, when this happened, was to return a slice ofDestinationKeyRange
keyspace IDs, which is not a single shard, thus failing fordelete equal
. This pull request changes this behavior by transformingdelete unique
intodelete scatter
when the query is made using a lookup Vindex onwrite_only
mode.Related Issue(s)
Checklist