Merge #81145

81145: sql: minor sqlsmith and tlp cleanup r=mgartner a=mgartner

#### sqlsmith: remove duplicate disableImpureFuncs

In #79973, `Smither.disableImpureFuncs` and `Smither.DisableImpureFuncs`
was added. The `Smither` already had `disableImpureFns` and
`DisableImpureFns` for disabling the generation of impure function
calls, so the newly added field and method are redundant. This commit
consolidates things.

Release note: None

#### tlp: use Smither.DisableImpureFns to disable impure functions

TLP now uses `Smither.DisableImpureFns` to disable the generation of
impure function calls, rather than writing to the internal
`Smither.disableImpureFns` field. Writing to the internal field directly
was preferred prior to #81017 when a single Smither was shared between
TLP query generation and mutation statement generation so that mutation
statements could use impure functions.

Release note: None


Co-authored-by: Marcus Gartner <[email protected]>

pkg/cmd/roachtest/tests/costfuzz.go

@@ -145,7 +145,7 @@ func runOneRoundCostFuzz(
 
 	// Initialize a smither that generates only deterministic SELECT statements.
 	smither, err := sqlsmith.NewSmither(conn, rnd,
-		sqlsmith.DisableMutations(), sqlsmith.DisableImpureFuncs(), sqlsmith.DisableLimits(),
+		sqlsmith.DisableMutations(), sqlsmith.DisableImpureFns(), sqlsmith.DisableLimits(),
 		sqlsmith.SetComplexity(.3),
 	)
 	if err != nil {

pkg/cmd/roachtest/tests/tlp.go

@@ -146,7 +146,8 @@ func runOneTLP(
 	defer mutSmither.Close()
 
 	// Initialize a smither that will never generate mutations.
-	tlpSmither, err := sqlsmith.NewSmither(conn, rnd, sqlsmith.DisableMutations())
+	tlpSmither, err := sqlsmith.NewSmither(conn, rnd,
+		sqlsmith.DisableMutations(), sqlsmith.DisableImpureFns())
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/internal/sqlsmith/scalar.go

@@ -372,7 +372,7 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
 	if class == tree.WindowClass && s.d6() != 1 {
 		class = tree.NormalClass
 	}
-	fns := s.functions[class][typ.Oid()]
+	fns := functions[class][typ.Oid()]
 	if len(fns) == 0 {
 		return nil, false
 	}

pkg/internal/sqlsmith/schema.go

@@ -23,7 +23,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treebin"
-	"github.com/cockroachdb/cockroach/pkg/sql/sem/volatility"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/errors"
 	"github.com/lib/pq/oid"
@@ -478,7 +477,7 @@ type function struct {
 	overload *tree.Overload
 }
 
-func functions(s *Smither) map[tree.FunctionClass]map[oid.Oid][]function {
+var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
 	m := map[tree.FunctionClass]map[oid.Oid][]function{}
 	for _, def := range tree.FunDefs {
 		switch def.Name {
@@ -511,19 +510,6 @@ func functions(s *Smither) map[tree.FunctionClass]map[oid.Oid][]function {
 		if skip {
 			continue
 		}
-		if s.disableImpureFuncs {
-			var impure bool
-			for _, def := range def.Definition {
-				overload := def.(*tree.Overload)
-				switch overload.Volatility {
-				case volatility.Stable, volatility.Volatile:
-					impure = true
-				}
-			}
-			if impure {
-				continue
-			}
-		}
 		if _, ok := m[def.Class]; !ok {
 			m[def.Class] = map[oid.Oid][]function{}
 		}
@@ -557,4 +543,4 @@ func functions(s *Smither) map[tree.FunctionClass]map[oid.Oid][]function {
 		}
 	}
 	return m
-}
+}()

pkg/internal/sqlsmith/sqlsmith.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
-	"github.com/lib/pq/oid"
 )
 
 // sqlsmith-go
@@ -69,7 +68,6 @@ type Smither struct {
 	nameCounts       map[string]int
 	activeSavepoints []string
 	types            *typeInfo
-	functions        map[tree.FunctionClass]map[oid.Oid][]function
 
 	stmtWeights, alterWeights          []statementWeight
 	stmtSampler, alterSampler          *statementSampler
@@ -84,7 +82,6 @@ type Smither struct {
 	disableImpureFns   bool
 	disableLimits      bool
 	disableWindowFuncs bool
-	disableImpureFuncs bool
 	simpleDatums       bool
 	avoidConsts        bool
 	outputSort         bool
@@ -125,7 +122,6 @@ func NewSmither(db *gosql.DB, rnd *rand.Rand, opts ...SmitherOption) (*Smither,
 	for _, opt := range opts {
 		opt.Apply(s)
 	}
-	s.functions = functions(s)
 	s.stmtSampler = newWeightedStatementSampler(s.stmtWeights, rnd.Int63())
 	s.alterSampler = newWeightedStatementSampler(s.alterWeights, rnd.Int63())
 	s.tableExprSampler = newWeightedTableExprSampler(s.tableExprWeights, rnd.Int63())
@@ -243,12 +239,6 @@ var DisableMutations = simpleOption("disable mutations", func(s *Smither) {
 	s.tableExprWeights = nonMutatingTableExprs
 })
 
-// DisableImpureFuncs causes the Smither to not emit statements that contain
-// impure builtins (aka builtins that don't always return the same result).
-var DisableImpureFuncs = simpleOption("disable impure funcs", func(s *Smither) {
-	s.disableImpureFuncs = true
-})
-
 // SetComplexity configures the Smither's complexity, in other words the
 // likelihood that at any given node the Smither will recurse and create a
 // deeper query true. The default is .2.

pkg/internal/sqlsmith/tlp.go

@@ -59,13 +59,6 @@ SELECT (SELECT count(*) FROM undiff), (SELECT count(*) FROM diff)`,
 // have all been implemented in SQLancer. See:
 // https://github.com/sqlancer/sqlancer/tree/1.1.0/src/sqlancer/cockroachdb/oracle/tlp.
 func (s *Smither) GenerateTLP() (unpartitioned, partitioned string, args []interface{}) {
-	// Set disableImpureFns to true so that generated predicates are immutable.
-	originalDisableImpureFns := s.disableImpureFns
-	s.disableImpureFns = true
-	defer func() {
-		s.disableImpureFns = originalDisableImpureFns
-	}()
-
 	switch tlpType := s.rnd.Intn(5); tlpType {
 	case 0:
 		partitioned, unpartitioned, args = s.generateWhereTLP()