From 4a61683637255887a4b7a8c4e36597486a746fd4 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Wed, 6 Mar 2019 15:07:45 -0500
Subject: [PATCH 01/23] sqlsmith: make db optional

This makes testing easier.
---
 pkg/internal/sqlsmith/sqlsmith.go      |  8 ++++++--
 pkg/internal/sqlsmith/sqlsmith_test.go | 13 ++-----------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go
index e2470736f1af..121e99e566ec 100644
--- a/pkg/internal/sqlsmith/sqlsmith.go
+++ b/pkg/internal/sqlsmith/sqlsmith.go
@@ -62,13 +62,17 @@ type Smither struct {
 	nameCounts map[string]int
 }
 
-// NewSmither creates a new Smither.
+// 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) {
 	s := &Smither{
 		rnd:        rnd,
 		nameCounts: map[string]int{},
 	}
-	err := s.ReloadSchemas(db)
+	var err error
+	if db != nil {
+		err = s.ReloadSchemas(db)
+	}
 	return s, err
 }
 
diff --git a/pkg/internal/sqlsmith/sqlsmith_test.go b/pkg/internal/sqlsmith/sqlsmith_test.go
index 674de9cc63ed..803d4f52c9fd 100644
--- a/pkg/internal/sqlsmith/sqlsmith_test.go
+++ b/pkg/internal/sqlsmith/sqlsmith_test.go
@@ -15,27 +15,18 @@
 package sqlsmith
 
 import (
-	gosql "database/sql"
-	"fmt"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
-	_ "github.com/lib/pq"
 )
 
 func TestGenerate(t *testing.T) {
-	t.Skip("used in local dev only")
-
-	db, err := gosql.Open("postgres", "user=root port=26257 sslmode=disable")
-	if err != nil {
-		t.Fatal(err)
-	}
 	rnd, _ := randutil.NewPseudoRand()
-	smither, err := NewSmither(db, rnd)
+	smither, err := NewSmither(nil /* db */, rnd)
 	if err != nil {
 		t.Fatal(err)
 	}
 	for i := 0; i < 5; i++ {
-		fmt.Println(smither.Generate())
+		_ = smither.Generate()
 	}
 }

From 73fa88240a1e35ef9f6c43ef1881db5b6de67e7f Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 01:39:25 -0500
Subject: [PATCH 02/23] sqlsmith: teach values how to return colRefs

Release note: None
---
 pkg/internal/sqlsmith/relational.go | 41 ++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index a953a26a9ae5..ddf3ea94c1f9 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -299,7 +299,7 @@ func (s *scope) makeInsertReturning(
 
 func (s *scope) makeValues(
 	desiredTypes []types.T, refs colRefs,
-) (*tree.ValuesClause, colRefs, bool) {
+) (*tree.SelectClause, colRefs, bool) {
 	if desiredTypes == nil {
 		for {
 			desiredTypes = append(desiredTypes, getRandType())
@@ -325,9 +325,44 @@ func (s *scope) makeValues(
 		}
 		values.Rows[i] = tuple
 	}
+	table := tree.Name(s.schema.name("tab"))
+	names := make(tree.NameList, len(desiredTypes))
+	valuesRefs := make(colRefs, len(desiredTypes))
+	for i, typ := range desiredTypes {
+		names[i] = tree.Name(s.schema.name("col"))
+		valuesRefs[i] = &colRef{
+			typ: typ,
+			item: tree.NewColumnItem(
+				tree.NewUnqualifiedTableName(table),
+				names[i],
+			),
+		}
+	}
 
-	// TODO(mjibson): figure out if we can return colRefs here.
-	return &values, nil, true
+	// Returing 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
+	// tbl (c1, c2, etc.)` to achieve this. There's quite a lot of indirection
+	// for how to achieve exactly that syntax as tree nodes, but it works.
+	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,
+				},
+			}},
+		},
+	}, valuesRefs, true
 }
 
 var setOps = []tree.UnionType{

From 8b4843dd7827be9fba6979517062d90b4720e3e6 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 01:43:13 -0500
Subject: [PATCH 03/23] sqlsmith: return tree.Name in namer

Release note: None
---
 pkg/internal/sqlsmith/relational.go | 10 +++++-----
 pkg/internal/sqlsmith/schema.go     |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index ddf3ea94c1f9..d86c403c5ec8 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -62,7 +62,7 @@ func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool
 		return nil, nil, nil, false
 	}
 	table := s.schema.tables[rand.Intn(len(s.schema.tables))]
-	alias := tree.Name(s.schema.name("tab"))
+	alias := s.schema.name("tab")
 	refs := make(colRefs, len(table.Columns))
 	for i, c := range table.Columns {
 		refs[i] = &colRef{
@@ -211,7 +211,7 @@ func (s *scope) makeSelectList(
 		result[i].As = tree.UnrestrictedName(alias)
 		selectRefs[i] = &colRef{
 			typ:  t,
-			item: &tree.ColumnItem{ColumnName: tree.Name(alias)},
+			item: &tree.ColumnItem{ColumnName: alias},
 		}
 	}
 	return result, selectRefs, true
@@ -287,7 +287,7 @@ func (s *scope) makeInsertReturning(
 		returningRefs[i] = &colRef{
 			typ: t,
 			item: &tree.ColumnItem{
-				ColumnName: tree.Name(alias),
+				ColumnName: alias,
 			},
 		}
 	}
@@ -325,11 +325,11 @@ func (s *scope) makeValues(
 		}
 		values.Rows[i] = tuple
 	}
-	table := tree.Name(s.schema.name("tab"))
+	table := s.schema.name("tab")
 	names := make(tree.NameList, len(desiredTypes))
 	valuesRefs := make(colRefs, len(desiredTypes))
 	for i, typ := range desiredTypes {
-		names[i] = tree.Name(s.schema.name("col"))
+		names[i] = s.schema.name("col")
 		valuesRefs[i] = &colRef{
 			typ: typ,
 			item: tree.NewColumnItem(
diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
index cacdf6379eea..89a79c376ab1 100644
--- a/pkg/internal/sqlsmith/schema.go
+++ b/pkg/internal/sqlsmith/schema.go
@@ -39,12 +39,12 @@ func (s *Smither) makeScope() *scope {
 	}
 }
 
-func (s *Smither) name(prefix string) string {
+func (s *Smither) name(prefix string) tree.Name {
 	s.lock.Lock()
 	s.nameCounts[prefix]++
 	count := s.nameCounts[prefix]
 	s.lock.Unlock()
-	return fmt.Sprintf("%s_%d", prefix, count)
+	return tree.Name(fmt.Sprintf("%s_%d", prefix, count))
 }
 
 // ReloadSchemas loads tables from the database. Not safe to use concurrently

From e6ccfea4e910029b9dc43f176920912aab7c046e Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 02:15:03 -0500
Subject: [PATCH 04/23] sqlsmith: add join types

Release note: None
---
 pkg/internal/sqlsmith/relational.go | 30 ++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index d86c403c5ec8..afabd665ae91 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -110,7 +110,14 @@ func (t typedExpr) ResolvedType() types.T {
 	return t.typ
 }
 
-// TODO(justin): also do outer joins.
+var joinTypes = []string{
+	"",
+	tree.AstFull,
+	tree.AstLeft,
+	tree.AstRight,
+	tree.AstCross,
+	tree.AstInner,
+}
 
 func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool) {
 	left, leftRefs, ok := s.makeDataSource(refs)
@@ -122,17 +129,22 @@ func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool)
 		return nil, nil, false
 	}
 
-	on, ok := s.makeBoolExpr(refs)
-	if !ok {
-		return nil, nil, false
+	joinExpr := &tree.JoinTableExpr{
+		JoinType: joinTypes[s.schema.rnd.Intn(len(joinTypes))],
+		Left:     left,
+		Right:    right,
+	}
+
+	if joinExpr.JoinType != tree.AstCross {
+		on, ok := s.makeBoolExpr(refs)
+		if !ok {
+			return nil, nil, false
+		}
+		joinExpr.Cond = &tree.OnJoinCond{Expr: on}
 	}
 	joinRefs := leftRefs.extend(rightRefs...)
 
-	return &tree.JoinTableExpr{
-		Left:  left,
-		Right: right,
-		Cond:  &tree.OnJoinCond{Expr: on},
-	}, joinRefs, true
+	return joinExpr, joinRefs, true
 }
 
 // STATEMENTS

From c05ee70814eb1fe2a36f516dcae5407df2e1557f Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 02:15:19 -0500
Subject: [PATCH 05/23] sqlsmith: ignore pg_sleep

Release note: None
---
 pkg/internal/sqlsmith/schema.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
index 89a79c376ab1..4007f90a7301 100644
--- a/pkg/internal/sqlsmith/schema.go
+++ b/pkg/internal/sqlsmith/schema.go
@@ -167,6 +167,10 @@ type function struct {
 var functions = func() map[oid.Oid][]function {
 	m := map[oid.Oid][]function{}
 	for _, def := range tree.FunDefs {
+		switch def.Name {
+		case "pg_sleep":
+			continue
+		}
 		if strings.Contains(def.Name, "crdb_internal.force_") {
 			continue
 		}

From 5b06352b582d57044a3a9bf4dc7876e5e5ab519a Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 02:29:17 -0500
Subject: [PATCH 06/23] sqlsmith: use the scopes rand when available

---
 pkg/internal/sqlsmith/relational.go | 6 ++----
 pkg/internal/sqlsmith/scalar.go     | 8 +++-----
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index afabd665ae91..6e19bba11963 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -15,8 +15,6 @@
 package sqlsmith
 
 import (
-	"math/rand"
-
 	"github.com/cockroachdb/cockroach/pkg/sql/coltypes"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/types"
@@ -61,7 +59,7 @@ func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool
 	if len(s.schema.tables) == 0 {
 		return nil, nil, nil, false
 	}
-	table := s.schema.tables[rand.Intn(len(s.schema.tables))]
+	table := s.schema.tables[s.schema.rnd.Intn(len(s.schema.tables))]
 	alias := s.schema.name("tab")
 	refs := make(colRefs, len(table.Columns))
 	for i, c := range table.Columns {
@@ -404,7 +402,7 @@ func (s *scope) makeSetOp(desiredTypes []types.T, refs colRefs) (*tree.UnionClau
 	}
 
 	return &tree.UnionClause{
-		Type:  setOps[rand.Intn(len(setOps))],
+		Type:  setOps[s.schema.rnd.Intn(len(setOps))],
 		Left:  &tree.Select{Select: left},
 		Right: &tree.Select{Select: right},
 		All:   coin(),
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index 7f57a8dd2c97..3cd0bd9c9e38 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -15,8 +15,6 @@
 package sqlsmith
 
 import (
-	"math/rand"
-
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/types"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
@@ -163,7 +161,7 @@ func (s *scope) makeColRef(typ types.T, refs colRefs) (tree.TypedExpr, *colRef,
 	if len(cols) == 0 {
 		return nil, nil, false
 	}
-	col := cols[rand.Intn(len(cols))]
+	col := cols[s.schema.rnd.Intn(len(cols))]
 	return makeTypedExpr(
 		col.item,
 		col.typ,
@@ -178,7 +176,7 @@ func (s *scope) makeBinOp(typ types.T, refs colRefs) (*tree.BinaryExpr, bool) {
 	if len(ops) == 0 {
 		return nil, false
 	}
-	op := ops[rand.Intn(len(ops))]
+	op := ops[s.schema.rnd.Intn(len(ops))]
 
 	left, ok := s.makeScalar(op.LeftType, refs)
 	if !ok {
@@ -205,7 +203,7 @@ func (s *scope) makeFunc(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	if len(fns) == 0 {
 		return nil, false
 	}
-	fn := fns[rand.Intn(len(fns))]
+	fn := fns[s.schema.rnd.Intn(len(fns))]
 
 	args := make(tree.TypedExprs, 0)
 	for _, typ := range fn.overload.Types.Types() {

From 0149783f334fe6b86278c10841e1286d5490e952 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 02:49:24 -0500
Subject: [PATCH 07/23] sqlsmith: remove a done TODO

---
 pkg/internal/sqlsmith/scope.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/pkg/internal/sqlsmith/scope.go b/pkg/internal/sqlsmith/scope.go
index 64b059ae4ed6..29faaf89ebc9 100644
--- a/pkg/internal/sqlsmith/scope.go
+++ b/pkg/internal/sqlsmith/scope.go
@@ -21,8 +21,6 @@ import (
 
 // colRef refers to a named result column. If it is from a table, def is
 // populated.
-// TODO(mjibson): wrap this in a type somehow so that makeColRef can do
-// better searching.
 type colRef struct {
 	typ  types.T
 	item *tree.ColumnItem

From 9eb1f0659e1a26cff8d744b49390dfeb24502db3 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 03:32:27 -0500
Subject: [PATCH 08/23] sqlsmith: remove unsupported [ ] table expr in joins

---
 pkg/internal/sqlsmith/relational.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 6e19bba11963..e3f8511c37b5 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -77,7 +77,9 @@ func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool
 	}, table, refs, true
 }
 
-func (s *scope) makeDataSource(refs colRefs) (tree.TableExpr, colRefs, bool) {
+// makeDataSource returns a tableExpr. If forJoin is true the tableExpr is
+// valid to be used as a join reference.
+func (s *scope) makeDataSource(refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
 	s = s.push()
 
 	if s.level < 3+d6() {
@@ -85,7 +87,8 @@ func (s *scope) makeDataSource(refs colRefs) (tree.TableExpr, colRefs, bool) {
 			return s.makeJoinExpr(refs)
 		}
 	}
-	if s.level < 3+d6() && coin() {
+	// Joins support the [ ] syntax only if it specifies a table ID, not a statement source.
+	if !forJoin && s.level < 3+d6() && coin() {
 		return s.makeInsertReturning(nil, refs)
 	}
 	expr, _, refs, ok := s.getTableExpr()
@@ -118,11 +121,11 @@ var joinTypes = []string{
 }
 
 func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool) {
-	left, leftRefs, ok := s.makeDataSource(refs)
+	left, leftRefs, ok := s.makeDataSource(refs, true)
 	if !ok {
 		return nil, nil, false
 	}
-	right, rightRefs, ok := s.makeDataSource(refs)
+	right, rightRefs, ok := s.makeDataSource(refs, true)
 	if !ok {
 		return nil, nil, false
 	}
@@ -156,7 +159,7 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 
 	var from tree.TableExpr
 	var fromRefs colRefs
-	from, fromRefs, ok = s.makeDataSource(refs)
+	from, fromRefs, ok = s.makeDataSource(refs, false)
 	if !ok {
 		return nil, nil, false
 	}

From bc72bb6ce2a719aab86da07214fc642140652481 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 03:10:05 -0500
Subject: [PATCH 09/23] sqlsmith: add test to parse generated statements

Release note: None
---
 pkg/internal/sqlsmith/main_test.go     | 37 +++++++++++++++++
 pkg/internal/sqlsmith/sqlsmith_test.go | 56 ++++++++++++++++++++++++--
 2 files changed, 89 insertions(+), 4 deletions(-)
 create mode 100644 pkg/internal/sqlsmith/main_test.go

diff --git a/pkg/internal/sqlsmith/main_test.go b/pkg/internal/sqlsmith/main_test.go
new file mode 100644
index 000000000000..7c7391bc8992
--- /dev/null
+++ b/pkg/internal/sqlsmith/main_test.go
@@ -0,0 +1,37 @@
+// Copyright 2019 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package sqlsmith
+
+import (
+	"os"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/security/securitytest"
+	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
+)
+
+//go:generate ../../util/leaktest/add-leaktest.sh *_test.go
+
+func TestMain(m *testing.M) {
+	security.SetAssetLoader(securitytest.EmbeddedAssets)
+	randutil.SeedForTests()
+	serverutils.InitTestServerFactory(server.TestServerFactory)
+	serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
+	os.Exit(m.Run())
+}
diff --git a/pkg/internal/sqlsmith/sqlsmith_test.go b/pkg/internal/sqlsmith/sqlsmith_test.go
index 803d4f52c9fd..f28292f7b178 100644
--- a/pkg/internal/sqlsmith/sqlsmith_test.go
+++ b/pkg/internal/sqlsmith/sqlsmith_test.go
@@ -15,18 +15,66 @@
 package sqlsmith
 
 import (
+	"context"
+	"flag"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 )
 
-func TestGenerate(t *testing.T) {
+var (
+	flagExec = flag.Bool("ex", false, "execute (instead of just parse) generated statements")
+	flagNum  = flag.Int("num", 100, "number of statements to generate")
+)
+
+func init() {
+	flag.Parse()
+}
+
+// TestGenerateParse verifies that statements produced by Generate can be
+// parsed. This is useful because since we make AST nodes directly we can
+// sometimes put them into bad states that the parser would never do.
+func TestGenerateParse(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	ctx := context.Background()
+	s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	defer s.Stopper().Stop(ctx)
+
 	rnd, _ := randutil.NewPseudoRand()
-	smither, err := NewSmither(nil /* db */, rnd)
+
+	db := sqlutils.MakeSQLRunner(sqlDB)
+	for i := 0; i < 10; i++ {
+		create := sqlbase.RandCreateTable(rnd, i)
+		db.Exec(t, create.String())
+	}
+
+	smither, err := NewSmither(sqlDB, rnd)
 	if err != nil {
 		t.Fatal(err)
 	}
-	for i := 0; i < 5; i++ {
-		_ = smither.Generate()
+	seen := map[string]bool{}
+	for i := 0; i < *flagNum; i++ {
+		stmt := smither.Generate()
+		_, err := parser.ParseOne(stmt)
+		t.Logf("%s;\n", stmt)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if *flagExec {
+			if _, err := sqlDB.Exec(stmt); err != nil {
+				es := err.Error()
+				if !seen[es] {
+					seen[es] = true
+					t.Errorf("ERR: %v\nSTATEMENT:\n%s;\n", err, stmt)
+				}
+			}
+		}
 	}
 }

From 25ed4906f3eabd4cebe3570c4399a720ea54b8ad Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Fri, 8 Mar 2019 16:02:15 -0500
Subject: [PATCH 10/23] sqlsmith: support DEFAULT VALUES in INSERT

---
 pkg/internal/sqlsmith/relational.go | 27 ++++++++++++++++-----------
 pkg/internal/sqlsmith/scalar.go     |  3 ---
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index e3f8511c37b5..c21404010c54 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -255,19 +255,24 @@ func (s *scope) makeInsert(refs colRefs) (*tree.Insert, *tableRef, bool) {
 		}
 	}
 
-	input, _, ok := s.makeReturningStmt(desiredTypes, refs)
-	if !ok {
-		return nil, nil, false
+	insert := &tree.Insert{
+		Table:     table,
+		Rows:      &tree.Select{},
+		Returning: &tree.NoReturningClause{},
 	}
 
-	return &tree.Insert{
-		Table:   table,
-		Columns: names,
-		Rows: &tree.Select{
-			Select: input,
-		},
-		Returning: &tree.NoReturningClause{},
-	}, tableRef, true
+	// Use DEFAULT VALUES only sometimes. A nil insert.Rows.Select indicates
+	// DEFAULT VALUES.
+	if d9() != 1 {
+		input, _, ok := s.makeReturningStmt(desiredTypes, refs)
+		if !ok {
+			return nil, nil, false
+		}
+		insert.Columns = names
+		insert.Rows.Select = input
+	}
+
+	return insert, tableRef, true
 }
 
 func (s *scope) makeInsertReturning(
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index 3cd0bd9c9e38..749ff6fa9a4c 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -144,9 +144,6 @@ func (s *scope) makeConstExpr(typ types.T) tree.TypedExpr {
 		s.schema.lock.Unlock()
 	}
 
-	// TODO(justin): maintain context and see if we're in an INSERT, and maybe use
-	// DEFAULT (which is a legal "value" in such a context).
-
 	return datum
 }
 

From dd5dc05bb511dac3b23f8a587f03812e1cbc4243 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Sat, 9 Mar 2019 00:44:15 -0500
Subject: [PATCH 11/23] sqlsmith: support non-named INSERT clauses

---
 pkg/internal/sqlsmith/relational.go | 36 +++++++++++++++++------------
 pkg/internal/sqlsmith/schema.go     | 10 +++++---
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index c21404010c54..a4bdd6942533 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -241,20 +241,6 @@ func (s *scope) makeInsert(refs colRefs) (*tree.Insert, *tableRef, bool) {
 		return nil, nil, false
 	}
 
-	var desiredTypes []types.T
-	var names tree.NameList
-
-	// Grab some subset of the columns of the table to attempt to insert into.
-	// TODO(justin): also support the non-named variant.
-	for _, c := range tableRef.Columns {
-		// We *must* write a column if it's writable and non-nullable.
-		// We *can* write a column if it's writable and nullable.
-		if !c.Computed.Computed && (c.Nullable.Nullability == tree.NotNull || coin()) {
-			desiredTypes = append(desiredTypes, coltypes.CastTargetToDatumType(c.Type))
-			names = append(names, c.Name)
-		}
-	}
-
 	insert := &tree.Insert{
 		Table:     table,
 		Rows:      &tree.Select{},
@@ -264,11 +250,31 @@ func (s *scope) makeInsert(refs colRefs) (*tree.Insert, *tableRef, bool) {
 	// Use DEFAULT VALUES only sometimes. A nil insert.Rows.Select indicates
 	// DEFAULT VALUES.
 	if d9() != 1 {
+		var desiredTypes []types.T
+		var names tree.NameList
+
+		unnamed := coin()
+
+		// Grab some subset of the columns of the table to attempt to insert into.
+		for _, c := range tableRef.Columns {
+			// We *must* write a column if it's writable and non-nullable.
+			// We *can* write a column if it's writable and nullable.
+			if c.Computed.Computed {
+				continue
+			}
+			if unnamed || c.Nullable.Nullability == tree.NotNull || coin() {
+				desiredTypes = append(desiredTypes, coltypes.CastTargetToDatumType(c.Type))
+				names = append(names, c.Name)
+			}
+		}
+
 		input, _, ok := s.makeReturningStmt(desiredTypes, refs)
 		if !ok {
 			return nil, nil, false
 		}
-		insert.Columns = names
+		if !unnamed {
+			insert.Columns = names
+		}
 		insert.Rows.Select = input
 	}
 
diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
index 4007f90a7301..167660898cff 100644
--- a/pkg/internal/sqlsmith/schema.go
+++ b/pkg/internal/sqlsmith/schema.go
@@ -64,7 +64,8 @@ SELECT
 	column_name,
 	crdb_sql_type,
 	generation_expression != '' AS computed,
-	is_nullable = 'YES' AS nullable
+	is_nullable = 'YES' AS nullable,
+	is_hidden = 'YES' AS hidden
 FROM
 	information_schema.columns
 WHERE
@@ -98,10 +99,13 @@ ORDER BY
 	for rows.Next() {
 		var catalog, schema, name, col tree.Name
 		var typ string
-		var computed, nullable bool
-		if err := rows.Scan(&catalog, &schema, &name, &col, &typ, &computed, &nullable); err != nil {
+		var computed, nullable, hidden bool
+		if err := rows.Scan(&catalog, &schema, &name, &col, &typ, &computed, &nullable, &hidden); err != nil {
 			return nil, err
 		}
+		if hidden {
+			continue
+		}
 
 		if firstTime {
 			lastCatalog = catalog

From 9e4aa8ed36514d2511654309b34998572d225023 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Sat, 9 Mar 2019 00:52:58 -0500
Subject: [PATCH 12/23] sqlsmith: add ORDER BY to SELECT

---
 pkg/internal/sqlsmith/relational.go | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index a4bdd6942533..26afa9d5aa7d 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -150,6 +150,11 @@ func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool)
 
 // STATEMENTS
 
+var orderDirections = []tree.Direction{
+	tree.Ascending,
+	tree.Descending,
+}
+
 func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select, colRefs, bool) {
 	var clause tree.SelectClause
 	var ok bool
@@ -179,18 +184,12 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 		clause.Where = tree.NewWhere("WHERE", where)
 	}
 
-	// TODO(justin): This can error a lot because it will often generate ORDER
-	// BY's like `order by 'foo'`, which is invalid. The only constant that can
-	// appear in ORDER BY is an integer and it must refer to a column ordinal. We
-	// should make it so the only constants it generates are integers less than
-	// the number of columns (or just disallow constants).
-	//for coin() {
-	//	expr, ok := outScope.makeScalar(anyType)
-	//	if !ok {
-	//		return nil, false
-	//	}
-	//	out.orderBy = append(out.orderBy, expr)
-	//}
+	for coin() {
+		stmt.OrderBy = append(stmt.OrderBy, &tree.Order{
+			Expr:      fromRefs[s.schema.rnd.Intn(len(fromRefs))].item,
+			Direction: orderDirections[s.schema.rnd.Intn(len(orderDirections))],
+		})
+	}
 
 	clause.Distinct = d100() == 1
 

From eff2d64c9936f0aaee49eba45b1626268dc9361e Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Sat, 9 Mar 2019 01:10:59 -0500
Subject: [PATCH 13/23] sqlsmith: use INSERT table cols in RETURNING

---
 pkg/internal/sqlsmith/relational.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 26afa9d5aa7d..e68cb4d54a13 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -292,15 +292,22 @@ func (s *scope) makeInsertReturning(
 		}
 	}
 
-	insert, _, ok := s.makeInsert(refs)
+	insert, insertRef, ok := s.makeInsert(refs)
 	if !ok {
 		return nil, nil, false
 	}
+	insertRefs := make(colRefs, len(insertRef.Columns))
+	for i, c := range insertRef.Columns {
+		insertRefs[i] = &colRef{
+			typ:  coltypes.CastTargetToDatumType(c.Type),
+			item: &tree.ColumnItem{ColumnName: c.Name},
+		}
+	}
 
 	returning := make(tree.ReturningExprs, len(desiredTypes))
 	returningRefs := make(colRefs, len(desiredTypes))
 	for i, t := range desiredTypes {
-		e, ok := s.makeScalar(t, refs)
+		e, ok := s.makeScalar(t, insertRefs)
 		if !ok {
 			return nil, nil, false
 		}

From c4ebc01ed1752fede714a154eb24c5591670e8ef Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Sat, 9 Mar 2019 02:03:02 -0500
Subject: [PATCH 14/23] sqlsmith: wrap BinOps in ParenExpr

Needed so that when multiple BinOps are adjacent the intended resolution
order is executed instead of depending on operator precedence.
---
 pkg/internal/sqlsmith/scalar.go | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index 749ff6fa9a4c..cf39b9db3ed7 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -165,7 +165,7 @@ func (s *scope) makeColRef(typ types.T, refs colRefs) (tree.TypedExpr, *colRef,
 	), col, true
 }
 
-func (s *scope) makeBinOp(typ types.T, refs colRefs) (*tree.BinaryExpr, bool) {
+func (s *scope) makeBinOp(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	if typ == types.Any {
 		typ = getRandType()
 	}
@@ -184,12 +184,13 @@ func (s *scope) makeBinOp(typ types.T, refs colRefs) (*tree.BinaryExpr, bool) {
 		return nil, false
 	}
 
-	return tree.NewTypedBinaryExpr(
-		op.Operator,
-		left,
-		right,
-		typ,
-	), true
+	return &tree.ParenExpr{
+		Expr: &tree.BinaryExpr{
+			Operator: op.Operator,
+			Left:     left,
+			Right:    right,
+		},
+	}, true
 }
 
 func (s *scope) makeFunc(typ types.T, refs colRefs) (tree.TypedExpr, bool) {

From 66b9a95319ac55b9cab291bbd0babff01f46dfee Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Sun, 10 Mar 2019 20:04:37 -0400
Subject: [PATCH 15/23] sqlsmith: refactor makeScalar to use weighted sampling

Refactor makeScalar and friends to use a weighted sampling algorithm to
generate scalars in a deterministic and pluggable way. This will allow
for much easier expansion and weighting of chosen methods.

Instead of teaching the sampling about some kind of funcy level falloff,
use a very simple function to determine if we can recurse into a more
complicated scalar generation function. For now this works great.

Maintain makeBoolExpr with separate weights because maybe the original
sqlsmith had a good reason for it.

Add makeExists to the top level makeScalar list so it can sometimes
generate bools when requested.
---
 pkg/internal/sqlsmith/random.go        |   8 --
 pkg/internal/sqlsmith/relational.go    |  10 +-
 pkg/internal/sqlsmith/sampler.go       |  66 +++++++++++
 pkg/internal/sqlsmith/scalar.go        | 157 ++++++++++++++-----------
 pkg/internal/sqlsmith/schema.go        |  15 ---
 pkg/internal/sqlsmith/scope.go         |   7 ++
 pkg/internal/sqlsmith/sqlsmith.go      |  26 +++-
 pkg/internal/sqlsmith/sqlsmith_test.go |  17 +++
 pkg/internal/sqlsmith/type.go          |   8 ++
 9 files changed, 212 insertions(+), 102 deletions(-)
 create mode 100644 pkg/internal/sqlsmith/sampler.go

diff --git a/pkg/internal/sqlsmith/random.go b/pkg/internal/sqlsmith/random.go
index 7d03d501140b..4e9e35732ae1 100644
--- a/pkg/internal/sqlsmith/random.go
+++ b/pkg/internal/sqlsmith/random.go
@@ -28,14 +28,6 @@ func d9() int {
 	return rand.Intn(6) + 1
 }
 
-func d20() int {
-	return rand.Intn(20) + 1
-}
-
-func d42() int {
-	return rand.Intn(42) + 1
-}
-
 func d100() int {
 	return rand.Intn(100) + 1
 }
diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index e68cb4d54a13..829b24f701e9 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -137,7 +137,7 @@ func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool)
 	}
 
 	if joinExpr.JoinType != tree.AstCross {
-		on, ok := s.makeBoolExpr(refs)
+		on, ok := makeBoolExpr(s, refs)
 		if !ok {
 			return nil, nil, false
 		}
@@ -177,7 +177,7 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 	clause.Exprs = selectList
 
 	if coin() {
-		where, ok := s.makeBoolExpr(refs)
+		where, ok := makeBoolExpr(s, refs)
 		if !ok {
 			return nil, nil, false
 		}
@@ -214,7 +214,7 @@ func (s *scope) makeSelectList(
 	result := make(tree.SelectExprs, len(desiredTypes))
 	selectRefs := make(colRefs, len(desiredTypes))
 	for i, t := range desiredTypes {
-		next, ok := s.makeScalar(t, refs)
+		next, ok := makeScalar(s, t, refs)
 		if !ok {
 			return nil, nil, false
 		}
@@ -307,7 +307,7 @@ func (s *scope) makeInsertReturning(
 	returning := make(tree.ReturningExprs, len(desiredTypes))
 	returningRefs := make(colRefs, len(desiredTypes))
 	for i, t := range desiredTypes {
-		e, ok := s.makeScalar(t, insertRefs)
+		e, ok := makeScalar(s, t, insertRefs)
 		if !ok {
 			return nil, nil, false
 		}
@@ -347,7 +347,7 @@ func (s *scope) makeValues(
 	for i := 0; i < numRowsToInsert; i++ {
 		tuple := make([]tree.Expr, len(desiredTypes))
 		for j, t := range desiredTypes {
-			e, ok := s.makeScalar(t, refs)
+			e, ok := makeScalar(s, t, refs)
 			if !ok {
 				return nil, nil, false
 			}
diff --git a/pkg/internal/sqlsmith/sampler.go b/pkg/internal/sqlsmith/sampler.go
new file mode 100644
index 000000000000..564140793288
--- /dev/null
+++ b/pkg/internal/sqlsmith/sampler.go
@@ -0,0 +1,66 @@
+// Copyright 2019 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package sqlsmith
+
+import (
+	"math/rand"
+
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
+)
+
+// NewWeightedSampler creates a WeightedSampler that produces indexes
+// corresponding to positions in weights. They are returned at the relative
+// frequency of the values of weights. For example, if weights is {1, 3},
+// reading 4 values from Next would return `0` one time and `1` three times
+// in some random order. All weights must be >= 1.
+func NewWeightedSampler(weights []int, seed int64) *WeightedSampler {
+	sum := 0
+	for _, w := range weights {
+		if w < 1 {
+			panic("expected weight >= 1")
+		}
+		sum += w
+	}
+	if sum == 0 {
+		panic("expected weights")
+	}
+	samples := make([]int, sum)
+	pos := 0
+	for wi, w := range weights {
+		for count := 0; count < w; count++ {
+			samples[pos] = wi
+			pos++
+		}
+	}
+	return &WeightedSampler{
+		rnd:     rand.New(rand.NewSource(seed)),
+		samples: samples,
+	}
+}
+
+// WeightedSampler is a weighted sampler.
+type WeightedSampler struct {
+	mu      syncutil.Mutex
+	rnd     *rand.Rand
+	samples []int
+}
+
+// Next returns the next weighted sample index.
+func (w *WeightedSampler) Next() int {
+	w.mu.Lock()
+	v := w.samples[w.rnd.Intn(len(w.samples))]
+	w.mu.Unlock()
+	return v
+}
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index cf39b9db3ed7..8f06c540819c 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -20,83 +20,87 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 )
 
-// makeScalar attempts to construct a scalar expression of the requested type.
-// If it was unsuccessful, it will return false.
-func (s *scope) makeScalar(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
-	s = s.push()
+var (
+	scalars, bools             []scalarWeight
+	scalarWeights, boolWeights []int
+)
 
-	pickedType := typ
-	if typ == types.Any {
-		pickedType = getRandType()
-	}
-
-	for i := 0; i < retryCount; i++ {
-		var result tree.TypedExpr
-		var ok bool
-		// TODO(justin): this is how sqlsmith chooses what to do, but it feels
-		// to me like there should be a more clean/principled approach here.
-		if s.level < d6() && d9() == 1 {
-			result, ok = s.makeCaseExpr(pickedType, refs)
-		} else if s.level < d6() && d42() == 1 {
-			result, ok = s.makeCoalesceExpr(pickedType, refs)
-		} else if len(refs) > 0 && d20() > 1 {
-			result, _, ok = s.makeColRef(typ, refs)
-		} else if s.level < d6() && d9() == 1 {
-			result, ok = s.makeBinOp(typ, refs)
-		} else if s.level < d6() && d9() == 1 {
-			result, ok = s.makeFunc(typ, refs)
-		} else if s.level < d6() && d6() == 1 {
-			result, ok = s.makeScalarSubquery(typ, refs)
-		} else {
-			result, ok = s.makeConstExpr(pickedType), true
-		}
-		if ok {
-			return result, ok
-		}
+func init() {
+	scalars = []scalarWeight{
+		{5, makeCaseExpr},
+		{1, makeCoalesceExpr},
+		{20, makeColRef},
+		{10, makeBinOp},
+		{10, makeFunc},
+		{2, makeScalarSubquery},
+		{2, makeExists},
+		{10, makeConstExpr},
 	}
+	scalarWeights = extractWeights(scalars)
 
-	// Retried enough times, give up.
-	return nil, false
+	bools = []scalarWeight{
+		{3, makeBinOp},
+		{2, makeScalar},
+		{1, makeExists},
+	}
+	boolWeights = extractWeights(bools)
 }
 
-// TODO(justin): sqlsmith separated this out from the general case for
-// some reason - I think there must be a clean way to unify the two.
-func (s *scope) makeBoolExpr(refs colRefs) (tree.TypedExpr, bool) {
-	s = s.push()
+type scalarWeight struct {
+	weight int
+	fn     func(*scope, types.T, colRefs) (expr tree.TypedExpr, ok bool)
+}
 
-	for i := 0; i < retryCount; i++ {
-		var result tree.TypedExpr
-		var ok bool
+func extractWeights(weights []scalarWeight) []int {
+	w := make([]int, len(weights))
+	for i, s := range weights {
+		w[i] = s.weight
+	}
+	return w
+}
 
-		if d6() < 4 {
-			result, ok = s.makeBinOp(types.Bool, refs)
-		} else if d6() < 4 {
-			result, ok = s.makeScalar(types.Bool, refs)
-		} else {
-			result, ok = s.makeExists(refs)
-		}
+// makeScalar attempts to construct a scalar expression of the requested type.
+// If it was unsuccessful, it will return false.
+func makeScalar(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	return makeScalarSample(s.schema.scalars, scalars, s, typ, refs)
+}
+
+func makeBoolExpr(s *scope, refs colRefs) (tree.TypedExpr, bool) {
+	return makeScalarSample(s.schema.bools, bools, s, types.Bool, refs)
+}
 
-		if ok {
-			return result, ok
+func makeScalarSample(
+	sampler *WeightedSampler, weights []scalarWeight, s *scope, typ types.T, refs colRefs,
+) (tree.TypedExpr, bool) {
+	s = s.push()
+	if s.canRecurse() {
+		for {
+			// No need for a retry counter here because makeConstExpr well eventually
+			// be called and it always succeeds.
+			idx := sampler.Next()
+			result, ok := weights[idx].fn(s, typ, refs)
+			if ok {
+				return result, ok
+			}
 		}
 	}
-
-	// Retried enough times, give up.
-	return nil, false
+	return makeConstExpr(s, typ, refs)
 }
 
-func (s *scope) makeCaseExpr(typ types.T, refs colRefs) (*tree.CaseExpr, bool) {
-	condition, ok := s.makeScalar(types.Bool, refs)
+func makeCaseExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	typ = pickAnyType(typ)
+
+	condition, ok := makeScalar(s, types.Bool, refs)
 	if !ok {
 		return nil, false
 	}
 
-	trueExpr, ok := s.makeScalar(typ, refs)
+	trueExpr, ok := makeScalar(s, typ, refs)
 	if !ok {
 		return nil, false
 	}
 
-	falseExpr, ok := s.makeScalar(typ, refs)
+	falseExpr, ok := makeScalar(s, typ, refs)
 	if !ok {
 		return nil, false
 	}
@@ -113,13 +117,15 @@ func (s *scope) makeCaseExpr(typ types.T, refs colRefs) (*tree.CaseExpr, bool) {
 	return expr, err == nil
 }
 
-func (s *scope) makeCoalesceExpr(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
-	firstExpr, ok := s.makeScalar(typ, refs)
+func makeCoalesceExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	typ = pickAnyType(typ)
+
+	firstExpr, ok := makeScalar(s, typ, refs)
 	if !ok {
 		return nil, false
 	}
 
-	secondExpr, ok := s.makeScalar(typ, refs)
+	secondExpr, ok := makeScalar(s, typ, refs)
 	if !ok {
 		return nil, false
 	}
@@ -133,7 +139,9 @@ func (s *scope) makeCoalesceExpr(typ types.T, refs colRefs) (tree.TypedExpr, boo
 	), true
 }
 
-func (s *scope) makeConstExpr(typ types.T) tree.TypedExpr {
+func makeConstExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	typ = pickAnyType(typ)
+
 	var datum tree.Datum
 	col, err := sqlbase.DatumTypeToColumnType(typ)
 	if err != nil {
@@ -144,10 +152,15 @@ func (s *scope) makeConstExpr(typ types.T) tree.TypedExpr {
 		s.schema.lock.Unlock()
 	}
 
-	return datum
+	return datum, true
+}
+
+func makeColRef(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	expr, _, ok := getColRef(s, typ, refs)
+	return expr, ok
 }
 
-func (s *scope) makeColRef(typ types.T, refs colRefs) (tree.TypedExpr, *colRef, bool) {
+func getColRef(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, *colRef, bool) {
 	// Filter by needed type.
 	cols := make(colRefs, 0, len(refs))
 	for _, c := range refs {
@@ -165,7 +178,7 @@ func (s *scope) makeColRef(typ types.T, refs colRefs) (tree.TypedExpr, *colRef,
 	), col, true
 }
 
-func (s *scope) makeBinOp(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+func makeBinOp(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	if typ == types.Any {
 		typ = getRandType()
 	}
@@ -175,11 +188,11 @@ func (s *scope) makeBinOp(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	}
 	op := ops[s.schema.rnd.Intn(len(ops))]
 
-	left, ok := s.makeScalar(op.LeftType, refs)
+	left, ok := makeScalar(s, op.LeftType, refs)
 	if !ok {
 		return nil, false
 	}
-	right, ok := s.makeScalar(op.RightType, refs)
+	right, ok := makeScalar(s, op.RightType, refs)
 	if !ok {
 		return nil, false
 	}
@@ -193,7 +206,7 @@ func (s *scope) makeBinOp(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	}, true
 }
 
-func (s *scope) makeFunc(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+func makeFunc(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	if typ == types.Any {
 		typ = getRandType()
 	}
@@ -205,7 +218,7 @@ func (s *scope) makeFunc(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 
 	args := make(tree.TypedExprs, 0)
 	for _, typ := range fn.overload.Types.Types() {
-		in, ok := s.makeScalar(typ, refs)
+		in, ok := makeScalar(s, typ, refs)
 		if !ok {
 			return nil, false
 		}
@@ -224,7 +237,11 @@ func (s *scope) makeFunc(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	), true
 }
 
-func (s *scope) makeExists(refs colRefs) (tree.TypedExpr, bool) {
+func makeExists(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+	if typ != types.Bool || typ != types.Any {
+		return nil, false
+	}
+
 	selectStmt, _, ok := s.makeSelect(nil, refs)
 	if !ok {
 		return nil, false
@@ -238,7 +255,7 @@ func (s *scope) makeExists(refs colRefs) (tree.TypedExpr, bool) {
 	return subq, true
 }
 
-func (s *scope) makeScalarSubquery(typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+func makeScalarSubquery(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	selectStmt, _, ok := s.makeSelect([]types.T{typ}, refs)
 	if !ok {
 		return nil, false
diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
index 167660898cff..5298be8bb4b2 100644
--- a/pkg/internal/sqlsmith/schema.go
+++ b/pkg/internal/sqlsmith/schema.go
@@ -16,7 +16,6 @@ package sqlsmith
 
 import (
 	gosql "database/sql"
-	"fmt"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/coltypes"
@@ -33,20 +32,6 @@ type tableRef struct {
 	Columns   []*tree.ColumnTableDef
 }
 
-func (s *Smither) makeScope() *scope {
-	return &scope{
-		schema: s,
-	}
-}
-
-func (s *Smither) name(prefix string) tree.Name {
-	s.lock.Lock()
-	s.nameCounts[prefix]++
-	count := s.nameCounts[prefix]
-	s.lock.Unlock()
-	return tree.Name(fmt.Sprintf("%s_%d", prefix, count))
-}
-
 // ReloadSchemas loads tables from the database. Not safe to use concurrently
 // with Generate.
 func (s *Smither) ReloadSchemas(db *gosql.DB) error {
diff --git a/pkg/internal/sqlsmith/scope.go b/pkg/internal/sqlsmith/scope.go
index 29faaf89ebc9..7841eea9ae01 100644
--- a/pkg/internal/sqlsmith/scope.go
+++ b/pkg/internal/sqlsmith/scope.go
@@ -49,3 +49,10 @@ func (s *scope) push() *scope {
 		schema: s.schema,
 	}
 }
+
+// canRecurse returns whether the current function should possibly invoke
+// a function that calls s.push. As the scope's level increases, the chance
+// for this function to return false increases.
+func (s *scope) canRecurse() bool {
+	return s.schema.rnd.Intn(5) > s.level
+}
diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go
index 121e99e566ec..f734a7c92b96 100644
--- a/pkg/internal/sqlsmith/sqlsmith.go
+++ b/pkg/internal/sqlsmith/sqlsmith.go
@@ -16,6 +16,7 @@ package sqlsmith
 
 import (
 	gosql "database/sql"
+	"fmt"
 	"math/rand"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -56,10 +57,11 @@ const retryCount = 20
 
 // Smither is a sqlsmith generator.
 type Smither struct {
-	rnd        *rand.Rand
-	lock       syncutil.Mutex
-	tables     []*tableRef
-	nameCounts map[string]int
+	rnd            *rand.Rand
+	lock           syncutil.Mutex
+	tables         []*tableRef
+	nameCounts     map[string]int
+	scalars, bools *WeightedSampler
 }
 
 // NewSmither creates a new Smither. db is used to populate existing tables
@@ -68,6 +70,8 @@ func NewSmither(db *gosql.DB, rnd *rand.Rand) (*Smither, error) {
 	s := &Smither{
 		rnd:        rnd,
 		nameCounts: map[string]int{},
+		scalars:    NewWeightedSampler(scalarWeights, rnd.Int63()),
+		bools:      NewWeightedSampler(boolWeights, rnd.Int63()),
 	}
 	var err error
 	if db != nil {
@@ -88,3 +92,17 @@ func (s *Smither) Generate() string {
 		return cfg.Pretty(stmt)
 	}
 }
+
+func (s *Smither) makeScope() *scope {
+	return &scope{
+		schema: s,
+	}
+}
+
+func (s *Smither) name(prefix string) tree.Name {
+	s.lock.Lock()
+	s.nameCounts[prefix]++
+	count := s.nameCounts[prefix]
+	s.lock.Unlock()
+	return tree.Name(fmt.Sprintf("%s_%d", prefix, count))
+}
diff --git a/pkg/internal/sqlsmith/sqlsmith_test.go b/pkg/internal/sqlsmith/sqlsmith_test.go
index f28292f7b178..38d6957e7908 100644
--- a/pkg/internal/sqlsmith/sqlsmith_test.go
+++ b/pkg/internal/sqlsmith/sqlsmith_test.go
@@ -17,6 +17,8 @@ package sqlsmith
 import (
 	"context"
 	"flag"
+	"math/rand"
+	"reflect"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
@@ -78,3 +80,18 @@ func TestGenerateParse(t *testing.T) {
 		}
 	}
 }
+
+func TestWeightedSampler(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	expected := []int{1, 1, 1, 1, 1, 0, 2, 2, 0, 0, 0, 1, 1, 2, 0, 2}
+
+	s := NewWeightedSampler([]int{1, 3, 4}, rand.New(rand.NewSource(0)))
+	var got []int
+	for i := 0; i < 16; i++ {
+		got = append(got, s.Next())
+	}
+	if !reflect.DeepEqual(expected, got) {
+		t.Fatalf("got %v, expected %v", got, expected)
+	}
+}
diff --git a/pkg/internal/sqlsmith/type.go b/pkg/internal/sqlsmith/type.go
index 998c4b21b95a..1d3fbf015d7c 100644
--- a/pkg/internal/sqlsmith/type.go
+++ b/pkg/internal/sqlsmith/type.go
@@ -51,3 +51,11 @@ func getRandType() types.T {
 	arr := types.AnyNonArray
 	return arr[rand.Intn(len(arr))]
 }
+
+// pickAnyType returns a concrete type if typ is types.Any, otherwise typ.
+func pickAnyType(typ types.T) types.T {
+	if typ == types.Any {
+		return getRandType()
+	}
+	return typ
+}

From ac78d1cbe22dac5c83de26fb7ab97b59d4ae0659 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 02:38:04 -0400
Subject: [PATCH 16/23] sqlsmith: convert data sources to weighted sampler

---
 pkg/internal/sqlsmith/relational.go | 67 ++++++++++++++++++++++-------
 pkg/internal/sqlsmith/sqlsmith.go   |  2 +
 2 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 829b24f701e9..427dbc7ecf6f 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -53,6 +53,11 @@ func (s *scope) makeReturningStmt(
 	return nil, nil, false
 }
 
+func getTableExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
+	expr, _, exprRefs, ok := s.getTableExpr()
+	return expr, exprRefs, ok
+}
+
 func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool) {
 	s = s.push()
 
@@ -77,22 +82,45 @@ func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool
 	}, table, refs, true
 }
 
+var (
+	dataSources   []sourceWeight
+	sourceWeights []int
+)
+
+func init() {
+	dataSources = []sourceWeight{
+		{2, makeJoinExpr},
+		{1, makeInsertReturning},
+		{3, getTableExpr},
+	}
+	sourceWeights = func() []int {
+		m := make([]int, len(dataSources))
+		for i, s := range dataSources {
+			m[i] = s.weight
+		}
+		return m
+	}()
+}
+
+type sourceWeight struct {
+	weight int
+	fn     func(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool)
+}
+
 // makeDataSource returns a tableExpr. If forJoin is true the tableExpr is
 // valid to be used as a join reference.
-func (s *scope) makeDataSource(refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
+func makeDataSource(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
 	s = s.push()
-
-	if s.level < 3+d6() {
-		if d6() > 4 {
-			return s.makeJoinExpr(refs)
+	if s.canRecurse() {
+		for i := 0; i < retryCount; i++ {
+			idx := s.schema.sources.Next()
+			expr, exprRefs, ok := dataSources[idx].fn(s, refs, forJoin)
+			if ok {
+				return expr, exprRefs, ok
+			}
 		}
 	}
-	// Joins support the [ ] syntax only if it specifies a table ID, not a statement source.
-	if !forJoin && s.level < 3+d6() && coin() {
-		return s.makeInsertReturning(nil, refs)
-	}
-	expr, _, refs, ok := s.getTableExpr()
-	return expr, refs, ok
+	return getTableExpr(s, refs, forJoin)
 }
 
 type typedExpr struct {
@@ -120,12 +148,12 @@ var joinTypes = []string{
 	tree.AstInner,
 }
 
-func (s *scope) makeJoinExpr(refs colRefs) (*tree.JoinTableExpr, colRefs, bool) {
-	left, leftRefs, ok := s.makeDataSource(refs, true)
+func makeJoinExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
+	left, leftRefs, ok := makeDataSource(s, refs, true)
 	if !ok {
 		return nil, nil, false
 	}
-	right, rightRefs, ok := s.makeDataSource(refs, true)
+	right, rightRefs, ok := makeDataSource(s, refs, true)
 	if !ok {
 		return nil, nil, false
 	}
@@ -164,7 +192,7 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 
 	var from tree.TableExpr
 	var fromRefs colRefs
-	from, fromRefs, ok = s.makeDataSource(refs, false)
+	from, fromRefs, ok = makeDataSource(s, refs, false)
 	if !ok {
 		return nil, nil, false
 	}
@@ -280,9 +308,16 @@ func (s *scope) makeInsert(refs colRefs) (*tree.Insert, *tableRef, bool) {
 	return insert, tableRef, true
 }
 
+func makeInsertReturning(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
+	if forJoin {
+		return nil, nil, false
+	}
+	return s.makeInsertReturning(nil, refs)
+}
+
 func (s *scope) makeInsertReturning(
 	desiredTypes []types.T, refs colRefs,
-) (*tree.StatementSource, colRefs, bool) {
+) (tree.TableExpr, colRefs, bool) {
 	if desiredTypes == nil {
 		for {
 			desiredTypes = append(desiredTypes, getRandType())
diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go
index f734a7c92b96..9fcd71669beb 100644
--- a/pkg/internal/sqlsmith/sqlsmith.go
+++ b/pkg/internal/sqlsmith/sqlsmith.go
@@ -62,6 +62,7 @@ type Smither struct {
 	tables         []*tableRef
 	nameCounts     map[string]int
 	scalars, bools *WeightedSampler
+	sources        *WeightedSampler
 }
 
 // NewSmither creates a new Smither. db is used to populate existing tables
@@ -72,6 +73,7 @@ func NewSmither(db *gosql.DB, rnd *rand.Rand) (*Smither, error) {
 		nameCounts: map[string]int{},
 		scalars:    NewWeightedSampler(scalarWeights, rnd.Int63()),
 		bools:      NewWeightedSampler(boolWeights, rnd.Int63()),
+		sources:    NewWeightedSampler(sourceWeights, rnd.Int63()),
 	}
 	var err error
 	if db != nil {

From 0ecb668a6409ea4bd94d99e8aca20400cd6b85df Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 03:27:35 -0400
Subject: [PATCH 17/23] sqlsmith: weighted sampling for data and returnings

---
 pkg/internal/sqlsmith/relational.go | 67 +++++++++++++++++++----------
 pkg/internal/sqlsmith/sqlsmith.go   | 13 +++---
 2 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 427dbc7ecf6f..1515122abc3d 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -34,23 +34,16 @@ func (s *scope) makeReturningStmt(
 ) (stmt tree.SelectStatement, stmtRefs colRefs, ok bool) {
 	s = s.push()
 
-	for i := 0; i < retryCount; i++ {
-		if s.level < d6() && d6() < 3 {
-			stmt, stmtRefs, ok = s.makeValues(desiredTypes, refs)
-		} else if s.level < d6() && d6() < 3 {
-			stmt, stmtRefs, ok = s.makeSetOp(desiredTypes, refs)
-		} else {
-			var inner *tree.Select
-			inner, stmtRefs, ok = s.makeSelect(desiredTypes, refs)
+	if s.canRecurse() {
+		for {
+			idx := s.schema.returnings.Next()
+			expr, exprRefs, ok := returnings[idx].fn(s, desiredTypes, refs)
 			if ok {
-				stmt = inner.Select
+				return expr, exprRefs, ok
 			}
 		}
-		if ok {
-			return stmt, stmtRefs, ok
-		}
 	}
-	return nil, nil, false
+	return makeValues(s, desiredTypes, refs)
 }
 
 func getTableExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
@@ -83,8 +76,9 @@ func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool
 }
 
 var (
-	dataSources   []sourceWeight
-	sourceWeights []int
+	dataSources                     []sourceWeight
+	returnings                      []returningWeight
+	sourceWeights, returningWeights []int
 )
 
 func init() {
@@ -100,12 +94,30 @@ func init() {
 		}
 		return m
 	}()
+	returnings = []returningWeight{
+		{1, makeValues},
+		{1, makeSetOp},
+		{1, makeSelect},
+	}
+	returningWeights = func() []int {
+		m := make([]int, len(returnings))
+		for i, s := range returnings {
+			m[i] = s.weight
+		}
+		return m
+	}()
 }
 
-type sourceWeight struct {
-	weight int
-	fn     func(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool)
-}
+type (
+	sourceWeight struct {
+		weight int
+		fn     func(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool)
+	}
+	returningWeight struct {
+		weight int
+		fn     func(s *scope, desiredTypes []types.T, refs colRefs) (tree.SelectStatement, colRefs, bool)
+	}
+)
 
 // makeDataSource returns a tableExpr. If forJoin is true the tableExpr is
 // valid to be used as a join reference.
@@ -183,6 +195,13 @@ var orderDirections = []tree.Direction{
 	tree.Descending,
 }
 
+func makeSelect(
+	s *scope, desiredTypes []types.T, refs colRefs,
+) (tree.SelectStatement, colRefs, bool) {
+	stmt, stmtRefs, ok := s.makeSelect(desiredTypes, refs)
+	return stmt.Select, stmtRefs, ok
+}
+
 func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select, colRefs, bool) {
 	var clause tree.SelectClause
 	var ok bool
@@ -362,9 +381,9 @@ func (s *scope) makeInsertReturning(
 	}, returningRefs, true
 }
 
-func (s *scope) makeValues(
-	desiredTypes []types.T, refs colRefs,
-) (*tree.SelectClause, colRefs, bool) {
+func makeValues(
+	s *scope, desiredTypes []types.T, refs colRefs,
+) (tree.SelectStatement, colRefs, bool) {
 	if desiredTypes == nil {
 		for {
 			desiredTypes = append(desiredTypes, getRandType())
@@ -436,7 +455,9 @@ var setOps = []tree.UnionType{
 	tree.ExceptOp,
 }
 
-func (s *scope) makeSetOp(desiredTypes []types.T, refs colRefs) (*tree.UnionClause, colRefs, bool) {
+func makeSetOp(
+	s *scope, desiredTypes []types.T, refs colRefs,
+) (tree.SelectStatement, colRefs, bool) {
 	if desiredTypes == nil {
 		for {
 			desiredTypes = append(desiredTypes, getRandType())
diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go
index 9fcd71669beb..fe5937dde5cd 100644
--- a/pkg/internal/sqlsmith/sqlsmith.go
+++ b/pkg/internal/sqlsmith/sqlsmith.go
@@ -57,12 +57,12 @@ const retryCount = 20
 
 // Smither is a sqlsmith generator.
 type Smither struct {
-	rnd            *rand.Rand
-	lock           syncutil.Mutex
-	tables         []*tableRef
-	nameCounts     map[string]int
-	scalars, bools *WeightedSampler
-	sources        *WeightedSampler
+	rnd                 *rand.Rand
+	lock                syncutil.Mutex
+	tables              []*tableRef
+	nameCounts          map[string]int
+	scalars, bools      *WeightedSampler
+	sources, returnings *WeightedSampler
 }
 
 // NewSmither creates a new Smither. db is used to populate existing tables
@@ -74,6 +74,7 @@ func NewSmither(db *gosql.DB, rnd *rand.Rand) (*Smither, error) {
 		scalars:    NewWeightedSampler(scalarWeights, rnd.Int63()),
 		bools:      NewWeightedSampler(boolWeights, rnd.Int63()),
 		sources:    NewWeightedSampler(sourceWeights, rnd.Int63()),
+		returnings: NewWeightedSampler(returningWeights, rnd.Int63()),
 	}
 	var err error
 	if db != nil {

From 2d3a41434934c6f9e45da838622b4826bd832330 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 03:27:55 -0400
Subject: [PATCH 18/23] sqlsmith: replace level and push concepts with a budget

Previously a hard coded concept of a level and pushing was used
to decrease chance of doing complicated things as statement depth
increased. This presented some problems like when to increase the level
and how to use the level when determining which thing to do.

Instead, make a single scope with a complexity budget that is decreased
every time we might do something that can add statement depth. This
causes most statements to eventually terminate in some leaf node. Since
the budget creation is random, this should allow for both larger and
smaller statements to be produced in general. Additionally, all of the
knobs (how much budget, how to decrease budget) are in fewer places,
making this more configurable.
---
 pkg/internal/sqlsmith/relational.go |  7 -------
 pkg/internal/sqlsmith/scalar.go     |  1 -
 pkg/internal/sqlsmith/scope.go      | 20 ++++++++++----------
 pkg/internal/sqlsmith/sqlsmith.go   |  6 ------
 4 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 1515122abc3d..300ff11e5f07 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -32,8 +32,6 @@ func (s *scope) makeStmt() (stmt tree.Statement, ok bool) {
 func (s *scope) makeReturningStmt(
 	desiredTypes []types.T, refs colRefs,
 ) (stmt tree.SelectStatement, stmtRefs colRefs, ok bool) {
-	s = s.push()
-
 	if s.canRecurse() {
 		for {
 			idx := s.schema.returnings.Next()
@@ -52,8 +50,6 @@ func getTableExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs
 }
 
 func (s *scope) getTableExpr() (*tree.AliasedTableExpr, *tableRef, colRefs, bool) {
-	s = s.push()
-
 	if len(s.schema.tables) == 0 {
 		return nil, nil, nil, false
 	}
@@ -122,7 +118,6 @@ type (
 // makeDataSource returns a tableExpr. If forJoin is true the tableExpr is
 // valid to be used as a join reference.
 func makeDataSource(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
-	s = s.push()
 	if s.canRecurse() {
 		for i := 0; i < retryCount; i++ {
 			idx := s.schema.sources.Next()
@@ -280,8 +275,6 @@ func (s *scope) makeSelectList(
 // used only in the optional returning section. Hence the irregular return
 // signature.
 func (s *scope) makeInsert(refs colRefs) (*tree.Insert, *tableRef, bool) {
-	s = s.push()
-
 	table, tableRef, _, ok := s.getTableExpr()
 	if !ok {
 		return nil, nil, false
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index 8f06c540819c..1422f9735dd9 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -72,7 +72,6 @@ func makeBoolExpr(s *scope, refs colRefs) (tree.TypedExpr, bool) {
 func makeScalarSample(
 	sampler *WeightedSampler, weights []scalarWeight, s *scope, typ types.T, refs colRefs,
 ) (tree.TypedExpr, bool) {
-	s = s.push()
 	if s.canRecurse() {
 		for {
 			// No need for a retry counter here because makeConstExpr well eventually
diff --git a/pkg/internal/sqlsmith/scope.go b/pkg/internal/sqlsmith/scope.go
index 7841eea9ae01..bcf56cf91772 100644
--- a/pkg/internal/sqlsmith/scope.go
+++ b/pkg/internal/sqlsmith/scope.go
@@ -37,22 +37,22 @@ func (t colRefs) extend(refs ...*colRef) colRefs {
 type scope struct {
 	schema *Smither
 
-	// level is how deep we are in the scope tree - it is used as a heuristic
-	// to eventually bottom out recursion (so we don't attempt to construct an
-	// infinitely large join, or something).
-	level int
+	// The budget tracks available complexity. It is randomly generated. Each
+	// call to canRecurse decreases it such that canRecurse will eventually
+	// always return false.
+	budget int
 }
 
-func (s *scope) push() *scope {
+func (s *Smither) makeScope() *scope {
 	return &scope{
-		level:  s.level + 1,
-		schema: s.schema,
+		schema: s,
+		budget: s.rnd.Intn(100),
 	}
 }
 
 // canRecurse returns whether the current function should possibly invoke
-// a function that calls s.push. As the scope's level increases, the chance
-// for this function to return false increases.
+// a function that calls creates new nodes.
 func (s *scope) canRecurse() bool {
-	return s.schema.rnd.Intn(5) > s.level
+	s.budget--
+	return s.budget > 0
 }
diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go
index fe5937dde5cd..7c562fd219ec 100644
--- a/pkg/internal/sqlsmith/sqlsmith.go
+++ b/pkg/internal/sqlsmith/sqlsmith.go
@@ -96,12 +96,6 @@ func (s *Smither) Generate() string {
 	}
 }
 
-func (s *Smither) makeScope() *scope {
-	return &scope{
-		schema: s,
-	}
-}
-
 func (s *Smither) name(prefix string) tree.Name {
 	s.lock.Lock()
 	s.nameCounts[prefix]++

From 5cfd3d63c3a1ab1ff5d993cb29bbc551be4177e2 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 03:39:10 -0400
Subject: [PATCH 19/23] sqlsmith: use from refs in where clause

---
 pkg/internal/sqlsmith/relational.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 300ff11e5f07..98eb183fa50b 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -219,7 +219,7 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 	clause.Exprs = selectList
 
 	if coin() {
-		where, ok := makeBoolExpr(s, refs)
+		where, ok := makeBoolExpr(s, fromRefs)
 		if !ok {
 			return nil, nil, false
 		}

From 295e39aa9a03ba0a8f68017301b766c3e53188ba Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 03:45:33 -0400
Subject: [PATCH 20/23] sqlsmith: fix ORDER BY in SELECT DISTINCT

For SELECT DISTINCT, ORDER BY expressions must appear in select list.
---
 pkg/internal/sqlsmith/relational.go | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 98eb183fa50b..47c1bf79889d 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -226,15 +226,20 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 		clause.Where = tree.NewWhere("WHERE", where)
 	}
 
+	orderByRefs := fromRefs
+	if d100() == 1 {
+		// For SELECT DISTINCT, ORDER BY expressions must appear in select list.
+		clause.Distinct = true
+		orderByRefs = selectRefs
+	}
+
 	for coin() {
 		stmt.OrderBy = append(stmt.OrderBy, &tree.Order{
-			Expr:      fromRefs[s.schema.rnd.Intn(len(fromRefs))].item,
+			Expr:      orderByRefs[s.schema.rnd.Intn(len(orderByRefs))].item,
 			Direction: orderDirections[s.schema.rnd.Intn(len(orderDirections))],
 		})
 	}
 
-	clause.Distinct = d100() == 1
-
 	if d6() > 2 {
 		stmt.Limit = &tree.Limit{Count: tree.NewDInt(tree.DInt(d100()))}
 	}

From 04c60f96e1971e780fec0293bb9468d668f349c1 Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 04:00:57 -0400
Subject: [PATCH 21/23] sqlsmith: test improvements

---
 pkg/internal/sqlsmith/sqlsmith_test.go | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/pkg/internal/sqlsmith/sqlsmith_test.go b/pkg/internal/sqlsmith/sqlsmith_test.go
index 38d6957e7908..1fb26955d585 100644
--- a/pkg/internal/sqlsmith/sqlsmith_test.go
+++ b/pkg/internal/sqlsmith/sqlsmith_test.go
@@ -17,13 +17,12 @@ package sqlsmith
 import (
 	"context"
 	"flag"
-	"math/rand"
+	"fmt"
 	"reflect"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
-	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -52,10 +51,14 @@ func TestGenerateParse(t *testing.T) {
 	rnd, _ := randutil.NewPseudoRand()
 
 	db := sqlutils.MakeSQLRunner(sqlDB)
-	for i := 0; i < 10; i++ {
-		create := sqlbase.RandCreateTable(rnd, i)
-		db.Exec(t, create.String())
-	}
+	db.Exec(t, `CREATE TABLE t (
+		i int,
+		f float,
+		d decimal,
+		s string,
+		b bytes,
+		z bool
+	)`)
 
 	smither, err := NewSmither(sqlDB, rnd)
 	if err != nil {
@@ -65,7 +68,6 @@ func TestGenerateParse(t *testing.T) {
 	for i := 0; i < *flagNum; i++ {
 		stmt := smither.Generate()
 		_, err := parser.ParseOne(stmt)
-		t.Logf("%s;\n", stmt)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -74,7 +76,7 @@ func TestGenerateParse(t *testing.T) {
 				es := err.Error()
 				if !seen[es] {
 					seen[es] = true
-					t.Errorf("ERR: %v\nSTATEMENT:\n%s;\n", err, stmt)
+					fmt.Printf("ERR (%d): %v\nSTATEMENT:\n%s;\n\n", i, err, stmt)
 				}
 			}
 		}
@@ -86,7 +88,7 @@ func TestWeightedSampler(t *testing.T) {
 
 	expected := []int{1, 1, 1, 1, 1, 0, 2, 2, 0, 0, 0, 1, 1, 2, 0, 2}
 
-	s := NewWeightedSampler([]int{1, 3, 4}, rand.New(rand.NewSource(0)))
+	s := NewWeightedSampler([]int{1, 3, 4}, 0)
 	var got []int
 	for i := 0; i < 16; i++ {
 		got = append(got, s.Next())

From babc9a91a6d85022a1fb56c0fff3d58ed1b9f07c Mon Sep 17 00:00:00 2001
From: Matt Jibson <matt.jibson@gmail.com>
Date: Mon, 11 Mar 2019 23:17:57 -0400
Subject: [PATCH 22/23] sqlsmith: remove bool return from makeScalar

It is guaranteed to succeed.
---
 pkg/internal/sqlsmith/relational.go | 28 +++---------
 pkg/internal/sqlsmith/scalar.go     | 70 +++++++++--------------------
 2 files changed, 25 insertions(+), 73 deletions(-)

diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
index 47c1bf79889d..b5ecd92c7cd4 100644
--- a/pkg/internal/sqlsmith/relational.go
+++ b/pkg/internal/sqlsmith/relational.go
@@ -172,10 +172,7 @@ func makeJoinExpr(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs
 	}
 
 	if joinExpr.JoinType != tree.AstCross {
-		on, ok := makeBoolExpr(s, refs)
-		if !ok {
-			return nil, nil, false
-		}
+		on := makeBoolExpr(s, refs)
 		joinExpr.Cond = &tree.OnJoinCond{Expr: on}
 	}
 	joinRefs := leftRefs.extend(rightRefs...)
@@ -219,10 +216,7 @@ func (s *scope) makeSelect(desiredTypes []types.T, refs colRefs) (*tree.Select,
 	clause.Exprs = selectList
 
 	if coin() {
-		where, ok := makeBoolExpr(s, fromRefs)
-		if !ok {
-			return nil, nil, false
-		}
+		where := makeBoolExpr(s, fromRefs)
 		clause.Where = tree.NewWhere("WHERE", where)
 	}
 
@@ -261,11 +255,7 @@ func (s *scope) makeSelectList(
 	result := make(tree.SelectExprs, len(desiredTypes))
 	selectRefs := make(colRefs, len(desiredTypes))
 	for i, t := range desiredTypes {
-		next, ok := makeScalar(s, t, refs)
-		if !ok {
-			return nil, nil, false
-		}
-		result[i].Expr = next
+		result[i].Expr = makeScalar(s, t, refs)
 		alias := s.schema.name("col")
 		result[i].As = tree.UnrestrictedName(alias)
 		selectRefs[i] = &colRef{
@@ -359,11 +349,7 @@ func (s *scope) makeInsertReturning(
 	returning := make(tree.ReturningExprs, len(desiredTypes))
 	returningRefs := make(colRefs, len(desiredTypes))
 	for i, t := range desiredTypes {
-		e, ok := makeScalar(s, t, insertRefs)
-		if !ok {
-			return nil, nil, false
-		}
-		returning[i].Expr = e
+		returning[i].Expr = makeScalar(s, t, insertRefs)
 		alias := s.schema.name("col")
 		returning[i].As = tree.UnrestrictedName(alias)
 		returningRefs[i] = &colRef{
@@ -399,11 +385,7 @@ func makeValues(
 	for i := 0; i < numRowsToInsert; i++ {
 		tuple := make([]tree.Expr, len(desiredTypes))
 		for j, t := range desiredTypes {
-			e, ok := makeScalar(s, t, refs)
-			if !ok {
-				return nil, nil, false
-			}
-			tuple[j] = e
+			tuple[j] = makeScalar(s, t, refs)
 		}
 		values.Rows[i] = tuple
 	}
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
index 1422f9735dd9..204fcbd90bd4 100644
--- a/pkg/internal/sqlsmith/scalar.go
+++ b/pkg/internal/sqlsmith/scalar.go
@@ -34,13 +34,17 @@ func init() {
 		{10, makeFunc},
 		{2, makeScalarSubquery},
 		{2, makeExists},
-		{10, makeConstExpr},
+		{10, func(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+			return makeConstExpr(s, typ, refs), true
+		}},
 	}
 	scalarWeights = extractWeights(scalars)
 
 	bools = []scalarWeight{
 		{3, makeBinOp},
-		{2, makeScalar},
+		{2, func(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+			return makeScalar(s, typ, refs), true
+		}},
 		{1, makeExists},
 	}
 	boolWeights = extractWeights(bools)
@@ -61,17 +65,17 @@ func extractWeights(weights []scalarWeight) []int {
 
 // makeScalar attempts to construct a scalar expression of the requested type.
 // If it was unsuccessful, it will return false.
-func makeScalar(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+func makeScalar(s *scope, typ types.T, refs colRefs) tree.TypedExpr {
 	return makeScalarSample(s.schema.scalars, scalars, s, typ, refs)
 }
 
-func makeBoolExpr(s *scope, refs colRefs) (tree.TypedExpr, bool) {
+func makeBoolExpr(s *scope, refs colRefs) tree.TypedExpr {
 	return makeScalarSample(s.schema.bools, bools, s, types.Bool, refs)
 }
 
 func makeScalarSample(
 	sampler *WeightedSampler, weights []scalarWeight, s *scope, typ types.T, refs colRefs,
-) (tree.TypedExpr, bool) {
+) tree.TypedExpr {
 	if s.canRecurse() {
 		for {
 			// No need for a retry counter here because makeConstExpr well eventually
@@ -79,7 +83,7 @@ func makeScalarSample(
 			idx := sampler.Next()
 			result, ok := weights[idx].fn(s, typ, refs)
 			if ok {
-				return result, ok
+				return result
 			}
 		}
 	}
@@ -88,22 +92,9 @@ func makeScalarSample(
 
 func makeCaseExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	typ = pickAnyType(typ)
-
-	condition, ok := makeScalar(s, types.Bool, refs)
-	if !ok {
-		return nil, false
-	}
-
-	trueExpr, ok := makeScalar(s, typ, refs)
-	if !ok {
-		return nil, false
-	}
-
-	falseExpr, ok := makeScalar(s, typ, refs)
-	if !ok {
-		return nil, false
-	}
-
+	condition := makeScalar(s, types.Bool, refs)
+	trueExpr := makeScalar(s, typ, refs)
+	falseExpr := makeScalar(s, typ, refs)
 	expr, err := tree.NewTypedCaseExpr(
 		nil,
 		[]*tree.When{{
@@ -118,17 +109,8 @@ func makeCaseExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 
 func makeCoalesceExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 	typ = pickAnyType(typ)
-
-	firstExpr, ok := makeScalar(s, typ, refs)
-	if !ok {
-		return nil, false
-	}
-
-	secondExpr, ok := makeScalar(s, typ, refs)
-	if !ok {
-		return nil, false
-	}
-
+	firstExpr := makeScalar(s, typ, refs)
+	secondExpr := makeScalar(s, typ, refs)
 	return tree.NewTypedCoalesceExpr(
 		tree.TypedExprs{
 			firstExpr,
@@ -138,7 +120,7 @@ func makeCoalesceExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool
 	), true
 }
 
-func makeConstExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
+func makeConstExpr(s *scope, typ types.T, refs colRefs) tree.TypedExpr {
 	typ = pickAnyType(typ)
 
 	var datum tree.Datum
@@ -151,7 +133,7 @@ func makeConstExpr(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 		s.schema.lock.Unlock()
 	}
 
-	return datum, true
+	return datum
 }
 
 func makeColRef(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
@@ -186,16 +168,8 @@ func makeBinOp(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 		return nil, false
 	}
 	op := ops[s.schema.rnd.Intn(len(ops))]
-
-	left, ok := makeScalar(s, op.LeftType, refs)
-	if !ok {
-		return nil, false
-	}
-	right, ok := makeScalar(s, op.RightType, refs)
-	if !ok {
-		return nil, false
-	}
-
+	left := makeScalar(s, op.LeftType, refs)
+	right := makeScalar(s, op.RightType, refs)
 	return &tree.ParenExpr{
 		Expr: &tree.BinaryExpr{
 			Operator: op.Operator,
@@ -217,11 +191,7 @@ func makeFunc(s *scope, typ types.T, refs colRefs) (tree.TypedExpr, bool) {
 
 	args := make(tree.TypedExprs, 0)
 	for _, typ := range fn.overload.Types.Types() {
-		in, ok := makeScalar(s, typ, refs)
-		if !ok {
-			return nil, false
-		}
-		args = append(args, in)
+		args = append(args, makeScalar(s, typ, refs))
 	}
 
 	return tree.NewTypedFuncExpr(

From 2fb974f11b11dc078f8d14228c6f76d9991b143c Mon Sep 17 00:00:00 2001
From: Lucy Zhang <lucy-zhang@users.noreply.github.com>
Date: Tue, 12 Mar 2019 14:23:16 -0400
Subject: [PATCH 23/23] sql: add regression tests for memory accounting bug

Add regression tests for a bug where backfilling a computed column or a column
with a default value using a builtin function would cause a `memory budget
exceeded` error (#34901). This was fixed by #34935.

Release note: None
---
 pkg/sql/logictest/testdata/logic_test/computed | 16 ++++++++++++++++
 pkg/sql/logictest/testdata/logic_test/default  | 16 ++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/pkg/sql/logictest/testdata/logic_test/computed b/pkg/sql/logictest/testdata/logic_test/computed
index 82f9032b3dd2..0d04610390d3 100644
--- a/pkg/sql/logictest/testdata/logic_test/computed
+++ b/pkg/sql/logictest/testdata/logic_test/computed
@@ -746,3 +746,19 @@ CREATE TABLE x (
 
 query error value type decimal doesn't match type INT8 of column "a"
 INSERT INTO x VALUES(1.4)
+
+# Regression test for #34901: verify that builtins can be used in computed
+# column expressions without a "memory budget exceeded" error while backfilling
+statement ok
+CREATE TABLE t34901 (x STRING)
+
+statement ok
+INSERT INTO t34901 VALUES ('a')
+
+statement ok
+ALTER TABLE t34901 ADD COLUMN y STRING AS (concat(x, 'b')) STORED
+
+query TT
+SELECT * FROM t34901
+----
+a  ab
diff --git a/pkg/sql/logictest/testdata/logic_test/default b/pkg/sql/logictest/testdata/logic_test/default
index fec1be69f46a..932ca5068648 100644
--- a/pkg/sql/logictest/testdata/logic_test/default
+++ b/pkg/sql/logictest/testdata/logic_test/default
@@ -117,3 +117,19 @@ column_name  data_type  is_nullable  column_default  generation_expression  indi
 a            INT8       false        NULL            ·                      {primary}  false
 b            TIMESTAMP  true         NULL            ·                      {}         false
 c            INT8       true         NULL            ·                      {}         false
+
+# Regression test for #34901: verify that builtins can be used in default value
+# expressions without a "memory budget exceeded" error while backfilling
+statement ok
+CREATE TABLE t34901 (x STRING)
+
+statement ok
+INSERT INTO t34901 VALUES ('a')
+
+statement ok
+ALTER TABLE t34901 ADD COLUMN y STRING DEFAULT (concat('b', 'c'))
+
+query TT
+SELECT * FROM t34901
+----
+a  bc