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

Support vtctl OnlineDDL <keyspace> show <context> #7145

Merged
merged 7 commits into from Dec 22, 2020

Conversation

shlomi-noach
Copy link
Contributor

Backport

NO

Status

DRAFT

Description

This adds support for vtctl OnlineDDL <keyspace> show <context>, to show all migrations that share the same context. For example, someone runs vtctl ApplySchema -ddl_strategy "gh-ost" -sql "multiple; queries; here;"

Each DDL query in the above gets its own UUID, the migration ID used for tracking that migration. But also, all of the migrations in that command have the same migration context, some arbitrary string that identifies them.

BTW vtctl ApplySchema... does not actually return that migration context (yet?). It's known internally to whatever service issued the migration, and can be used to track status for those migrations as a whole. We may expose the migration context to the user in the future.

Related Issue(s)

#6926 (comment)
followup to #7082

Todos

  • Tests
  • Documentation

Impacted Areas in Vitess

List general components of the application that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
…ontext

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach requested a review from a team December 13, 2020 07:47
@shlomi-noach
Copy link
Contributor Author

Actually there's a SQL injection case here; I need to parse/bind the query first.

@shlomi-noach shlomi-noach marked this pull request as draft December 13, 2020 07:48
…ontext

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Contributor Author

I added a new convenience function in sqlparser:

func ParseAndBind(in string, binds ...*querypb.BindVariable) (query string, err error) 

which allows for quickly creating a binded query, e.g.:

query, err := ParseAndBind("select * from tbl where name=%a", sqltypes.StringBindVariable("it's me"))

And with that I fixed SQL injection concerns in vtctl/vtctl.go.

@shlomi-noach shlomi-noach marked this pull request as ready for review December 21, 2020 08:48
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

lgtm

@shlomi-noach shlomi-noach merged commit 883688a into vitessio:master Dec 22, 2020
@shlomi-noach shlomi-noach deleted the online-ddl-show-context branch December 22, 2020 11:59
@askdba askdba added this to the v9.0 milestone Dec 23, 2020
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.

None yet

3 participants