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.

Sign up for GitHub

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

Jump to bottom

Fixed a SplitQuery panic() when run on a table with no primary key columns (e.g. a view) #2332

Merged
merged 1 commit into from
Dec 2, 2016
Merged

Fixed a SplitQuery panic() when run on a table with no primary key columns (e.g. a view) #2332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions go/vt/tabletserver/splitquery/split_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ func newSplitParams(
var splitColumns []*schema.TableColumn
if len(splitColumnNames) == 0 {
splitColumns = getPrimaryKeyColumns(tableSchema)
if len(splitColumns) == 0 {
return nil, fmt.Errorf("no split columns where given and the queried table has"+
" no primary key columns (is the table a view? Running SplitQuery on a view"+
" is not supported). query: %v", sql)
}
} else {
splitColumns, err = findSplitColumnsInSchema(splitColumnNames, tableSchema)
if err != nil {
Expand Down
39 changes: 36 additions & 3 deletions go/vt/tabletserver/splitquery/split_params_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var splitParamsTestCases = []struct {
SplitColumnNames []sqlparser.ColIdent
NumRowsPerQueryPart int64
SplitCount int64
Schema map[string]*schema.Table

ExpectedErrorRegex *regexp.Regexp
ExpectedSplitParams SplitParams
Expand All @@ -24,6 +25,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
SplitCount: 100,
Schema: getTestSchema(),

ExpectedSplitParams: SplitParams{
splitCount: 100,
Expand All @@ -37,6 +39,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedSplitParams: SplitParams{
splitCount: 10,
Expand All @@ -49,6 +52,7 @@ var splitParamsTestCases = []struct {
SQL: "select user_id from test_table",
BindVariables: map[string]interface{}{"foo": "123"},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedSplitParams: SplitParams{
splitCount: 10,
Expand All @@ -66,6 +70,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("failed parsing query: 'not a valid query'"),
},
Expand All @@ -74,6 +79,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("not a select statement"),
},
Expand All @@ -82,6 +88,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported FROM clause"),
},
Expand All @@ -90,6 +97,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported query"),
},
Expand All @@ -98,6 +106,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported query"),
},
Expand All @@ -106,6 +115,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported query"),
},
Expand All @@ -114,6 +124,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported query"),
},
Expand All @@ -122,6 +133,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported query"),
},
Expand All @@ -130,6 +142,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("unsupported FROM clause"),
},
Expand All @@ -138,6 +151,7 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("id")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("can't find table in schema"),
},
Expand All @@ -146,16 +160,35 @@ var splitParamsTestCases = []struct {
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("missing_column")},
NumRowsPerQueryPart: 100,
ExpectedErrorRegex: regexp.MustCompile("can't find split column"),
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile("can't find split column"),
},
{ // Test NewSplitParamsGivenNumRowsPerQueryPart; split columns not a prefix of an index.
SQL: "select * from test_table",
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{sqlparser.NewColIdent("float32_col")},
NumRowsPerQueryPart: 100,
Schema: getTestSchema(),

ExpectedErrorRegex: regexp.MustCompile(
"split-columns must be a prefix of the columns composing an index"),
},
{ // Test NewSplitParamsGivenNumRowsPerQueryPart; no split columns and no primary keys.
SQL: "select id from test_table",
BindVariables: map[string]interface{}{"foo": "123"},
SplitColumnNames: []sqlparser.ColIdent{},
NumRowsPerQueryPart: 100,
Schema: func() map[string]*schema.Table {
// Returns the testSchema without the primary key columns.
result := getTestSchema()
result["test_table"].PKColumns = []int{}
return result
}(),

ExpectedErrorRegex: regexp.MustCompile(
"no split columns where given and the queried table has no primary key columns"),
},
}

func TestSplitParams(t *testing.T) {
Expand All @@ -168,14 +201,14 @@ func TestSplitParams(t *testing.T) {
testCase.BindVariables,
testCase.SplitColumnNames,
testCase.NumRowsPerQueryPart,
testSchema)
testCase.Schema)
} else {
splitParams, err = NewSplitParamsGivenSplitCount(
testCase.SQL,
testCase.BindVariables,
testCase.SplitColumnNames,
testCase.SplitCount,
testSchema)
testCase.Schema)
}
if testCase.ExpectedErrorRegex != nil {
if !testCase.ExpectedErrorRegex.MatchString(err.Error()) {
Expand Down