From f739921194c73f56ad75698d321546141fc51bd8 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Wed, 8 May 2019 17:34:46 -0400 Subject: [PATCH 1/6] sqlsmith: allow disabling mutations This will be used when we want to set some initial state and not have to worry about it. Useful for comparison testing across separate servers to avoid having to make sure changes are synced. Release note: None --- pkg/internal/sqlsmith/relational.go | 71 ++++++++++++++++------------- pkg/internal/sqlsmith/sqlsmith.go | 53 +++++++++++++++------ 2 files changed, 80 insertions(+), 44 deletions(-) diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go index e5003498c229..a0de66a72b30 100644 --- a/pkg/internal/sqlsmith/relational.go +++ b/pkg/internal/sqlsmith/relational.go @@ -23,8 +23,8 @@ import ( ) func (s *scope) makeStmt() (stmt tree.Statement, ok bool) { - idx := s.schema.stmts.Next() - return statements[idx].fn(s) + idx := s.schema.stmtSampler.Next() + return s.schema.statements[idx].fn(s) } func (s *scope) makeSelectStmt( @@ -75,42 +75,33 @@ func (s *scope) tableExpr(table *tableRef, name *tree.TableName) (tree.TableExpr } var ( - statements []statementWeight - tableExprs []tableExprWeight - selectStmts []selectStmtWeight - statementWeights []int - tableExprWeights, selectStmtWeights []int -) - -func init() { - statements = []statementWeight{ + mutatingStatements = statementWeights{ {10, makeInsert}, - {10, makeSelect}, {10, makeDelete}, {10, makeUpdate}, {1, makeAlter}, } - statementWeights = func() []int { - m := make([]int, len(statements)) - for i, s := range statements { - m[i] = s.weight - } - return m - }() - tableExprs = []tableExprWeight{ - {2, makeJoinExpr}, + nonMutatingStatements = statementWeights{ + {10, makeSelect}, + } + allStatements = append(mutatingStatements, nonMutatingStatements...) + + mutatingTableExprs = tableExprWeights{ {1, makeInsertReturning}, {1, makeDeleteReturning}, {1, makeUpdateReturning}, + } + nonMutatingTableExprs = tableExprWeights{ + {2, makeJoinExpr}, {3, makeSchemaTable}, } - tableExprWeights = func() []int { - m := make([]int, len(tableExprs)) - for i, s := range tableExprs { - m[i] = s.weight - } - return m - }() + allTableExprs = append(mutatingTableExprs, nonMutatingTableExprs...) + + selectStmts []selectStmtWeight + selectStmtWeights []int +) + +func init() { selectStmts = []selectStmtWeight{ {1, makeValues}, {1, makeSetOp}, @@ -125,15 +116,33 @@ func init() { }() } +func (ws statementWeights) Weights() []int { + m := make([]int, len(ws)) + for i, w := range ws { + m[i] = w.weight + } + return m +} + +func (ws tableExprWeights) Weights() []int { + m := make([]int, len(ws)) + for i, w := range ws { + m[i] = w.weight + } + return m +} + type ( statementWeight struct { weight int fn func(s *scope) (tree.Statement, bool) } - tableExprWeight struct { + statementWeights []statementWeight + tableExprWeight struct { weight int fn func(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) } + tableExprWeights []tableExprWeight selectStmtWeight struct { weight int fn selectStmt @@ -151,8 +160,8 @@ type ( func makeTableExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) { if s.canRecurse() { for i := 0; i < retryCount; i++ { - idx := s.schema.tableExprs.Next() - expr, exprRefs, ok := tableExprs[idx].fn(s, refs, forJoin) + idx := s.schema.tableExprSampler.Next() + expr, exprRefs, ok := s.schema.tableExprs[idx].fn(s, refs, forJoin) if ok { return expr, exprRefs, ok } diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go index 4f3e40b803b1..f9ec7f63796e 100644 --- a/pkg/internal/sqlsmith/sqlsmith.go +++ b/pkg/internal/sqlsmith/sqlsmith.go @@ -57,32 +57,41 @@ const retryCount = 20 // Smither is a sqlsmith generator. type Smither struct { - rnd *rand.Rand - db *gosql.DB - lock syncutil.RWMutex - tables []*tableRef - indexes map[tree.TableName]map[tree.Name]*tree.CreateIndex - nameCounts map[string]int - stmts *WeightedSampler - alters *WeightedSampler - scalars, bools *WeightedSampler - tableExprs, selectStmts *WeightedSampler + rnd *rand.Rand + db *gosql.DB + lock syncutil.RWMutex + tables []*tableRef + indexes map[tree.TableName]map[tree.Name]*tree.CreateIndex + nameCounts map[string]int + alters *WeightedSampler + scalars, bools *WeightedSampler + selectStmts *WeightedSampler + + stmtSampler, tableExprSampler *WeightedSampler + statements statementWeights + tableExprs tableExprWeights } // NewSmither creates a new Smither. db is used to populate existing tables // for use as column references. It can be nil to skip table population. -func NewSmither(db *gosql.DB, rnd *rand.Rand) (*Smither, error) { +func NewSmither(db *gosql.DB, rnd *rand.Rand, opts ...SmitherOption) (*Smither, error) { s := &Smither{ rnd: rnd, db: db, nameCounts: map[string]int{}, - stmts: NewWeightedSampler(statementWeights, rnd.Int63()), scalars: NewWeightedSampler(scalarWeights, rnd.Int63()), bools: NewWeightedSampler(boolWeights, rnd.Int63()), - tableExprs: NewWeightedSampler(tableExprWeights, rnd.Int63()), selectStmts: NewWeightedSampler(selectStmtWeights, rnd.Int63()), alters: NewWeightedSampler(alterWeights, rnd.Int63()), + + statements: allStatements, + tableExprs: allTableExprs, + } + for _, opt := range opts { + opt.Apply(s) } + s.stmtSampler = NewWeightedSampler(s.statements.Weights(), rnd.Int63()) + s.tableExprSampler = NewWeightedSampler(s.tableExprs.Weights(), rnd.Int63()) return s, s.ReloadSchemas() } @@ -112,3 +121,21 @@ func (s *Smither) name(prefix string) tree.Name { s.lock.Unlock() return tree.Name(fmt.Sprintf("%s_%d", prefix, count)) } + +// SmitherOption is an option for the Smither client. +type SmitherOption interface { + Apply(*Smither) +} + +// DisableMutations causes the Smither to not emit statements that could +// mutate any on-disk data. +func DisableMutations() SmitherOption { + return disableMutations{} +} + +type disableMutations struct{} + +func (d disableMutations) Apply(s *Smither) { + s.statements = nonMutatingStatements + s.tableExprs = nonMutatingTableExprs +} From afe7965cc33f60d2bbf019bdae1a9739622f5f4b Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 10 May 2019 15:19:51 -0400 Subject: [PATCH 2/6] sqlsmith: allow disabling WITHs --- pkg/internal/sqlsmith/relational.go | 4 ++++ pkg/internal/sqlsmith/sqlsmith.go | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go index a0de66a72b30..131c95fc0aac 100644 --- a/pkg/internal/sqlsmith/relational.go +++ b/pkg/internal/sqlsmith/relational.go @@ -223,6 +223,10 @@ func makeJoinExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs // STATEMENTS func (s *scope) makeWith() (*tree.With, tableRefs) { + if s.schema.disableWith { + return nil, nil + } + // WITHs are pretty rare, so just ignore them a lot. if coin() { return nil, nil diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go index f9ec7f63796e..99b528385d56 100644 --- a/pkg/internal/sqlsmith/sqlsmith.go +++ b/pkg/internal/sqlsmith/sqlsmith.go @@ -70,6 +70,8 @@ type Smither struct { stmtSampler, tableExprSampler *WeightedSampler statements statementWeights tableExprs tableExprWeights + + disableWith bool } // NewSmither creates a new Smither. db is used to populate existing tables @@ -139,3 +141,14 @@ func (d disableMutations) Apply(s *Smither) { s.statements = nonMutatingStatements s.tableExprs = nonMutatingTableExprs } + +// DisableWith causes the Smither to not emit WITH clauses. +func DisableWith() SmitherOption { + return disableWith{} +} + +type disableWith struct{} + +func (d disableWith) Apply(s *Smither) { + s.disableWith = true +} From 21f4871d29602312b25a19ad922f55a77c2c76c2 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 10 May 2019 15:21:35 -0400 Subject: [PATCH 3/6] sqlsmith: bail after too many failed attempts --- pkg/internal/sqlsmith/sqlsmith.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go index 99b528385d56..26cbd8771e45 100644 --- a/pkg/internal/sqlsmith/sqlsmith.go +++ b/pkg/internal/sqlsmith/sqlsmith.go @@ -106,12 +106,18 @@ var prettyCfg = func() tree.PrettyCfg { // Generate returns a random SQL string. func (s *Smither) Generate() string { + i := 0 for { scope := s.makeScope() stmt, ok := scope.makeStmt() if !ok { + i++ + if i > 1000 { + panic("exhausted generation attempts") + } continue } + i = 0 return prettyCfg.Pretty(stmt) } } From aaf72ad3c6561c2f2d38c613c0be3d6517052f65 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 10 May 2019 15:29:26 -0400 Subject: [PATCH 4/6] sqlsmith: make CASE more rare --- pkg/internal/sqlsmith/scalar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go index f6aacad137c6..be4d05d2b3d4 100644 --- a/pkg/internal/sqlsmith/scalar.go +++ b/pkg/internal/sqlsmith/scalar.go @@ -28,7 +28,7 @@ var ( func init() { scalars = []scalarWeight{ {10, scalarNoContext(makeAnd)}, - {5, scalarNoContext(makeCaseExpr)}, + {1, scalarNoContext(makeCaseExpr)}, {1, scalarNoContext(makeCoalesceExpr)}, {20, scalarNoContext(makeColRef)}, {10, scalarNoContext(makeBinOp)}, From 33f609e8eee7413632251da0e73a229bc169a3c2 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 10 May 2019 16:19:20 -0400 Subject: [PATCH 5/6] sqlsmith: SELECT about more table sources It can now use VALUES and SELECT as table sources. Also teach it about having no FROM sources at all. --- pkg/internal/sqlsmith/relational.go | 164 +++++++++++++++++++--------- 1 file changed, 111 insertions(+), 53 deletions(-) diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go index 131c95fc0aac..442628c9e7f6 100644 --- a/pkg/internal/sqlsmith/relational.go +++ b/pkg/internal/sqlsmith/relational.go @@ -39,7 +39,7 @@ func (s *scope) makeSelectStmt( } } } - return makeValues(s, desiredTypes, refs, withTables) + return makeValuesSelect(s, desiredTypes, refs, withTables) } func makeSchemaTable(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) { @@ -94,6 +94,8 @@ var ( nonMutatingTableExprs = tableExprWeights{ {2, makeJoinExpr}, {3, makeSchemaTable}, + {1, makeValuesTable}, + {1, makeSelectTable}, } allTableExprs = append(mutatingTableExprs, nonMutatingTableExprs...) @@ -103,7 +105,7 @@ var ( func init() { selectStmts = []selectStmtWeight{ - {1, makeValues}, + {1, makeValuesSelect}, {1, makeSetOp}, {1, makeSelectClause}, } @@ -331,6 +333,40 @@ func (s *Smither) randStringComparison() tree.ComparisonOperator { return stringComparisons[s.rnd.Intn(len(stringComparisons))] } +// makeSelectTable returns a TableExpr of the form `(SELECT ...)`, which +// would end up looking like `SELECT ... FROM (SELECT ...)`. +func makeSelectTable(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) { + desiredTypes := makeDesiredTypes(s.schema.rnd) + stmt, _, ok := s.makeSelect(desiredTypes, refs) + if !ok { + return nil, nil, false + } + + table := s.schema.name("tab") + names := make(tree.NameList, len(desiredTypes)) + clauseRefs := make(colRefs, len(desiredTypes)) + for i, typ := range desiredTypes { + names[i] = s.schema.name("col") + clauseRefs[i] = &colRef{ + typ: typ, + item: tree.NewColumnItem( + tree.NewUnqualifiedTableName(table), + names[i], + ), + } + } + + return &tree.AliasedTableExpr{ + Expr: &tree.Subquery{ + Select: &tree.ParenSelect{Select: stmt}, + }, + As: tree.AliasClause{ + Alias: table, + Cols: names, + }, + }, clauseRefs, true +} + func makeSelectClause( s *scope, desiredTypes []*types.T, refs colRefs, withTables tableRefs, ) (tree.SelectStatement, colRefs, tableRefs, bool) { @@ -346,7 +382,9 @@ func (s *scope) makeSelectClause( } var fromRefs colRefs - for len(clause.From.Tables) < 1 || coin() { + // Sometimes generate a SELECT with no FROM clause. + requireFrom := d6() != 1 + for (requireFrom && len(clause.From.Tables) < 1) || coin() { var from tree.TableExpr if len(withTables) == 0 || coin() { // Add a normal data source. @@ -366,42 +404,47 @@ func (s *scope) makeSelectClause( } clause.From.Tables = append(clause.From.Tables, from) } - clause.Where = s.makeWhere(fromRefs) - orderByRefs = fromRefs - selectListRefs := fromRefs + + selectListRefs := refs ctx := emptyCtx - if d6() <= 2 { - // Enable GROUP BY. Choose some random subset of the - // fromRefs. - // TODO(mjibson): Refence handling and aggregation functions - // aren't quite handled correctly here. This currently - // does well enough to at least find some bugs but should - // be improved to do the correct thing wrt aggregate - // functions. That is, the select and having exprs can - // either reference a group by column or a non-group by - // column in an aggregate function. It's also possible - // the where and order by exprs are not correct. - groupByRefs := fromRefs.extend() - s.schema.rnd.Shuffle(len(groupByRefs), func(i, j int) { - groupByRefs[i], groupByRefs[j] = groupByRefs[j], groupByRefs[i] - }) - var groupBy tree.GroupBy - for (len(groupBy) < 1 || coin()) && len(groupBy) < len(groupByRefs) { - groupBy = append(groupBy, groupByRefs[len(groupBy)].item) + if len(clause.From.Tables) > 0 { + clause.Where = s.makeWhere(fromRefs) + orderByRefs = fromRefs + selectListRefs = selectListRefs.extend(fromRefs...) + + if d6() <= 2 { + // Enable GROUP BY. Choose some random subset of the + // fromRefs. + // TODO(mjibson): Refence handling and aggregation functions + // aren't quite handled correctly here. This currently + // does well enough to at least find some bugs but should + // be improved to do the correct thing wrt aggregate + // functions. That is, the select and having exprs can + // either reference a group by column or a non-group by + // column in an aggregate function. It's also possible + // the where and order by exprs are not correct. + groupByRefs := fromRefs.extend() + s.schema.rnd.Shuffle(len(groupByRefs), func(i, j int) { + groupByRefs[i], groupByRefs[j] = groupByRefs[j], groupByRefs[i] + }) + var groupBy tree.GroupBy + for (len(groupBy) < 1 || coin()) && len(groupBy) < len(groupByRefs) { + groupBy = append(groupBy, groupByRefs[len(groupBy)].item) + } + groupByRefs = groupByRefs[:len(groupBy)] + clause.GroupBy = groupBy + clause.Having = s.makeHaving(fromRefs) + selectListRefs = groupByRefs + orderByRefs = groupByRefs + // TODO(mjibson): also use this context sometimes in + // non-aggregate mode (select sum(x) from a). + ctx = groupByCtx + } else if d6() <= 1 { + // Enable window functions. This will enable them for all + // exprs, but makeFunc will only let a few through. + ctx = windowCtx } - groupByRefs = groupByRefs[:len(groupBy)] - clause.GroupBy = groupBy - clause.Having = s.makeHaving(fromRefs) - selectListRefs = groupByRefs - orderByRefs = groupByRefs - // TODO(mjibson): also use this context sometimes in - // non-aggregate mode (select sum(x) from a). - ctx = groupByCtx - } else if d6() <= 1 { - // Enable window functions. This will enable them for all - // exprs, but makeFunc will only let a few through. - ctx = windowCtx } selectList, selectRefs, ok := s.makeSelectList(ctx, desiredTypes, selectListRefs) @@ -663,9 +706,13 @@ func (s *scope) makeInsertReturning(refs colRefs) (tree.TableExpr, colRefs, bool }, returningRefs, true } -func makeValues( - s *scope, desiredTypes []*types.T, refs colRefs, withTables tableRefs, -) (tree.SelectStatement, colRefs, tableRefs, bool) { +func makeValuesTable(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) { + types := makeDesiredTypes(s.schema.rnd) + values, valuesRefs := makeValues(s, types, refs) + return values, valuesRefs, true +} + +func makeValues(s *scope, desiredTypes []*types.T, refs colRefs) (tree.TableExpr, colRefs) { numRowsToInsert := d6() values := tree.ValuesClause{ Rows: make([]tree.Exprs, numRowsToInsert), @@ -692,7 +739,27 @@ func makeValues( } } - // Returing just &values here would result in a query like `VALUES (...)` where + return &tree.AliasedTableExpr{ + Expr: &tree.Subquery{ + Select: &tree.ParenSelect{ + Select: &tree.Select{ + Select: &values, + }, + }, + }, + As: tree.AliasClause{ + Alias: table, + Cols: names, + }, + }, valuesRefs +} + +func makeValuesSelect( + s *scope, desiredTypes []*types.T, refs colRefs, withTables tableRefs, +) (tree.SelectStatement, colRefs, tableRefs, bool) { + values, valuesRefs := makeValues(s, desiredTypes, refs) + + // Returning just &values here would result in a query like `VALUES (...)` where // the columns are arbitrarily named by index (column1, column2, etc.). Since // we want to be able to reference the columns in other places we need to // name them deterministically. We can use `SELECT * FROM (VALUES (...)) AS @@ -701,19 +768,7 @@ func makeValues( return &tree.SelectClause{ Exprs: tree.SelectExprs{tree.StarSelectExpr()}, From: &tree.From{ - Tables: tree.TableExprs{&tree.AliasedTableExpr{ - Expr: &tree.Subquery{ - Select: &tree.ParenSelect{ - Select: &tree.Select{ - Select: &values, - }, - }, - }, - As: tree.AliasClause{ - Alias: table, - Cols: names, - }, - }}, + Tables: tree.TableExprs{values}, }, }, valuesRefs, withTables, true } @@ -762,6 +817,9 @@ func (s *scope) makeHaving(refs colRefs) *tree.Where { } func (s *scope) makeOrderBy(refs colRefs) tree.OrderBy { + if len(refs) == 0 { + return nil + } var ob tree.OrderBy for coin() { ref := refs[s.schema.rnd.Intn(len(refs))] From a33a5cc326d0d95897167b67bf4cc5fc08e09ded Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 10 May 2019 16:21:28 -0400 Subject: [PATCH 6/6] sqlsmith: allow tests to turn off mutations and WITHs --- pkg/internal/sqlsmith/sqlsmith_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/internal/sqlsmith/sqlsmith_test.go b/pkg/internal/sqlsmith/sqlsmith_test.go index 16a95224ddd1..51c41fdc048a 100644 --- a/pkg/internal/sqlsmith/sqlsmith_test.go +++ b/pkg/internal/sqlsmith/sqlsmith_test.go @@ -30,8 +30,10 @@ import ( ) var ( - flagExec = flag.Bool("ex", false, "execute (instead of just parse) generated statements") - flagNum = flag.Int("num", 100, "number of statements to generate") + flagExec = flag.Bool("ex", false, "execute (instead of just parse) generated statements") + flagNum = flag.Int("num", 100, "number of statements to generate") + flagNoMutations = flag.Bool("no-mutations", false, "disables mutations during testing") + flagNoWiths = flag.Bool("no-withs", false, "disables WITHs during testing") ) func init() { @@ -73,7 +75,15 @@ func TestGenerateParse(t *testing.T) { INSERT INTO t DEFAULT VALUES; `) - smither, err := NewSmither(sqlDB, rnd) + var opts []SmitherOption + if *flagNoMutations { + opts = append(opts, DisableMutations()) + } + if *flagNoWiths { + opts = append(opts, DisableWith()) + } + + smither, err := NewSmither(nil, rnd, opts...) if err != nil { t.Fatal(err) }