Skip to content

Commit

Permalink
Two cherry-picks from upstream
Browse files Browse the repository at this point in the history
Summary:
1) Don't let one bad sequence disable sequences for other tables
vitessio#4743

2) Route "show create table" correctly
vitessio#4732

Reviewers: #vitess, hzhou, mingjianliu

Reviewed By: #vitess, mingjianliu

Subscribers: mingjianliu, jenkins

Differential Revision: https://phabricator.pinadmin.com/D382885
  • Loading branch information
dweitzman committed Apr 3, 2019
1 parent b092449 commit b552b7a
Show file tree
Hide file tree
Showing 8 changed files with 970 additions and 853 deletions.
10 changes: 10 additions & 0 deletions go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,7 @@ func (f *ForeignKeyDefinition) walkSubtree(visit Visit) error {
type Show struct {
Type string
OnTable TableName
Table TableName
ShowTablesOpt *ShowTablesOpt
Scope string
ShowCollationFilterOpt *Expr
Expand Down Expand Up @@ -1548,13 +1549,22 @@ func (node *Show) Format(buf *TrackedBuffer) {
if node.Type == "collation" && node.ShowCollationFilterOpt != nil {
buf.Myprintf(" where %v", *node.ShowCollationFilterOpt)
}
if node.HasTable() {
buf.Myprintf(" %v", node.Table)
}
}

// HasOnTable returns true if the show statement has an "on" clause
func (node *Show) HasOnTable() bool {
return node.OnTable.Name.v != ""
}

// HasTable returns true if the show statement has a parsed table name.
// Not all show statements parse table names.
func (node *Show) HasTable() bool {
return node.Table.Name.v != ""
}

func (node *Show) walkSubtree(visit Visit) error {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ var (
output: "show create procedure",
}, {
input: "show create table t",
output: "show create table",
output: "show create table t",
}, {
input: "show create trigger t",
output: "show create trigger",
Expand Down
1,720 changes: 873 additions & 847 deletions go/vt/sqlparser/sql.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions go/vt/sqlparser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -1486,9 +1486,9 @@ show_statement:
{
$$ = &Show{Type: string($2) + " " + string($3)}
}
| SHOW CREATE TABLE ddl_skip_to_end
| SHOW CREATE TABLE table_name
{
$$ = &Show{Type: string($2) + " " + string($3)}
$$ = &Show{Type: string($2) + " " + string($3), Table: $4}
}
| SHOW CREATE TRIGGER ddl_skip_to_end
{
Expand Down
19 changes: 19 additions & 0 deletions go/vt/vtgate/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,25 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql
Rows: rows,
RowsAffected: 2,
}, nil
case "create table":
if destKeyspace == "" && show.HasTable() {
// For "show create table", if there isn't a targeted keyspace already
// we can either get a keyspace from the statement or potentially from
// the vschema.

if !show.Table.Qualifier.IsEmpty() {
// Explicit keyspace was passed. Use that for targeting but remove from the query itself.
destKeyspace = show.Table.Qualifier.String()
show.Table.Qualifier = sqlparser.NewTableIdent("")
sql = sqlparser.String(show)
} else {
// No keyspace was indicated. Try to find one using the vschema.
tbl, err := e.VSchema().FindTable("", show.Table.Name.String())
if err == nil {
destKeyspace = tbl.Keyspace.Name
}
}
}
case sqlparser.KeywordString(sqlparser.TABLES):
if show.ShowTablesOpt != nil && show.ShowTablesOpt.DbName != "" {
if destKeyspace == "" {
Expand Down
28 changes: 28 additions & 0 deletions go/vt/vtgate/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,34 @@ func TestExecutorShow(t *testing.T) {
t.Errorf("%v:\n%+v, want\n%+v", query, qr, wantqr)
}

qr, err = executor.Execute(context.Background(), "TestExecute", session, "show create table unknown_table", nil)
if err != errNoKeyspace {
t.Errorf("Got: %v. Want: %v", err, errNoKeyspace)
}

// SHOW CREATE table using vschema to find keyspace.
_, err = executor.Execute(context.Background(), "TestExecute", session, "show create table user_seq", nil)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

lastQuery := sbclookup.Queries[len(sbclookup.Queries)-1].Sql
wantQuery := "show create table user_seq"
if lastQuery != wantQuery {
t.Errorf("Got: %v. Want: %v", lastQuery, wantQuery)
}

// SHOW CREATE table with query-provided keyspace
_, err = executor.Execute(context.Background(), "TestExecute", session, fmt.Sprintf("show create table %v.unknown", KsTestUnsharded), nil)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
lastQuery = sbclookup.Queries[len(sbclookup.Queries)-1].Sql
wantQuery = "show create table unknown"
if lastQuery != wantQuery {
t.Errorf("Got: %v. Want: %v", lastQuery, wantQuery)
}

for _, query := range []string{"show charset", "show charset like '%foo'", "show character set", "show character set like '%foo'"} {
qr, err := executor.Execute(context.Background(), "TestExecute", session, query, nil)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/vindexes/vschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func resolveAutoIncrement(source *vschemapb.SrvVSchema, vschema *VSchema) {
seq, err := vschema.findQualified(table.AutoIncrement.Sequence)
if err != nil {
ksvschema.Error = fmt.Errorf("cannot resolve sequence %s: %v", table.AutoIncrement.Sequence, err)
break
continue
}
t.AutoIncrement.Sequence = seq
}
Expand Down
38 changes: 36 additions & 2 deletions go/vt/vtgate/vindexes/vschema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,13 @@ func TestSequence(t *testing.T) {
func TestBadSequence(t *testing.T) {
bad := vschemapb.SrvVSchema{
Keyspaces: map[string]*vschemapb.Keyspace{
"unsharded": {
Tables: map[string]*vschemapb.Table{
"valid_seq": {
Type: "sequence",
},
},
},
"sharded": {
Sharded: true,
Vindexes: map[string]*vschemapb.Vindex{
Expand All @@ -1458,7 +1465,19 @@ func TestBadSequence(t *testing.T) {
},
AutoIncrement: &vschemapb.AutoIncrement{
Column: "c1",
Sequence: "seq",
Sequence: "invalid_seq",
},
},
"t2": {
ColumnVindexes: []*vschemapb.ColumnVindex{
{
Column: "c1",
Name: "stfu1",
},
},
AutoIncrement: &vschemapb.AutoIncrement{
Column: "c1",
Sequence: "valid_seq",
},
},
},
Expand All @@ -1467,10 +1486,25 @@ func TestBadSequence(t *testing.T) {
}
got, _ := BuildVSchema(&bad)
err := got.Keyspaces["sharded"].Error
want := "cannot resolve sequence seq: table seq not found"
want := "cannot resolve sequence invalid_seq: table invalid_seq not found"
if err == nil || err.Error() != want {
t.Errorf("BuildVSchema: %v, want %v", err, want)
}

t1Seq := got.Keyspaces["sharded"].Tables["t1"].AutoIncrement.Sequence
if t1Seq != nil {
t.Errorf("BuildVSchema: unexpected sequence for table t1: %v", t1Seq)
}

// Verify that a failure to set up a sequence for t1 doesn't prevent setting up
// a sequence for t2.
t2Seq := got.Keyspaces["sharded"].Tables["t2"].AutoIncrement.Sequence
if t2Seq.Name.String() != "valid_seq" {
t.Errorf("BuildVSchema: unexpected t2 sequence name. Got: %v. Want: %v",
t2Seq.AutoIncrement.Sequence.Name,
"valid_seq",
)
}
}

func TestBadSequenceName(t *testing.T) {
Expand Down

0 comments on commit b552b7a

Please sign in to comment.