Skip to content
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

Adds supports to target DestinationKeyspaceID to handle other #6040

Merged

Conversation

rafael
Copy link
Member

@rafael rafael commented Apr 9, 2020

Description

  • minor change: for handle other if the target is a keyspaceID, it should be safe to execute it because is by definition a single shard.
  • Use case for this is to be able to do queries like this:
     # Let's say we have a shard with two shards -40 ,  40-
     # Before this change, this works:
     use ks/-40
     show tables; 
     # But this does not work:
     use ks[00];
     show tables;
    
  • Given that a targeting to a keyspaceID is also a single shard, it seems like this should be supported.

Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
@rafael rafael requested a review from sougou as a code owner April 9, 2020 23:18
@rafael
Copy link
Member Author

rafael commented Apr 10, 2020

cc: @systay , @harshit-gangal - @sougou mentioned you might be interested in taking a look at this change too.

Copy link

@setassociative setassociative left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that this should be a safe routing change.

@sougou sougou merged commit 04f66d3 into vitessio:master Apr 10, 2020
@rafael rafael deleted the adds-dest-keyspace-id-to-handle-other branch April 10, 2020 19:21
ajm188 pushed a commit to tinyspeck/vitess that referenced this pull request Apr 28, 2020
…-to-handle-other

Adds supports to target DestinationKeyspaceID to handle other
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants