Merge cockroachdb#127652

127652: opt: set all session settings to their global default r=mgartner a=mgartner

#### opt: reformat benchmark slow-queries.sql

Release note: None

#### opt: set all session settings to their global default

All session settings in optimizer tests and benchmarks are now set to
their global defaults. Prior to this commit each new session setting had
to be manually set in both tests and benchmarks. This step could be
forgotten, such as with `optimizer_merge_joins_enabled` which was
unknowingly disabled in benchmarks when it was added. Now, all session
settings with global defaults are automatically set.

Additionally, `sql.SetSessionVariable` has been renamed to
`sql.TestingSetSessionVariable` to discourage its use outside of tests.

NOTE: Now that `optimizer_merge_joins_enabled` is enabled, some
benchmarks will appear to regress.

Epic: None

Release note: None

#### opt: enable implicit SELECT FOR UPDATE locking in opt tests

Implicit `SELECT ... FOR UPDATE` locking has been enabled in optimizer
tests, matching the default value for the corresponding session setting
`enable_implicit_select_for_update`.

Release note: None


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

pkg/sql/opt/bench/BUILD.bazel

@@ -24,9 +24,8 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/server",
         "//pkg/settings/cluster",
+        "//pkg/sql",
         "//pkg/sql/catalog/schemaexpr",
-        "//pkg/sql/catalog/tabledesc",
-        "//pkg/sql/opt",
         "//pkg/sql/opt/exec",
         "//pkg/sql/opt/exec/execbuilder",
         "//pkg/sql/opt/exec/explain",
@@ -44,5 +43,6 @@ go_test(
         "//pkg/testutils/sqlutils",
         "//pkg/util/log",
         "//pkg/util/randutil",
+        "@com_github_cockroachdb_errors//:errors",
     ],
 )

pkg/sql/opt/bench/bench_test.go

@@ -23,9 +23,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/security/securitytest"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
-	"github.com/cockroachdb/cockroach/pkg/sql/opt"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
@@ -43,6 +42,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
+	"github.com/cockroachdb/errors"
 )
 
 // A query can be issued using the "simple protocol" or the "prepare protocol".
@@ -754,25 +754,10 @@ func newHarness(tb testing.TB, query benchQuery, schemas []string) *harness {
 		evalCtx: eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings()),
 	}
 
-	// Setup the default session settings.
-	h.evalCtx.SessionData().OptimizerUseMultiColStats = true
-	h.evalCtx.SessionData().ZigzagJoinEnabled = true
-	h.evalCtx.SessionData().OptimizerUseForecasts = true
-	h.evalCtx.SessionData().OptimizerUseHistograms = true
-	h.evalCtx.SessionData().LocalityOptimizedSearch = true
-	h.evalCtx.SessionData().ReorderJoinsLimit = opt.DefaultJoinOrderLimit
-	h.evalCtx.SessionData().InsertFastPath = true
-	h.evalCtx.SessionData().OptSplitScanLimit = tabledesc.MaxBucketAllowed
-	h.evalCtx.SessionData().VariableInequalityLookupJoinEnabled = true
-	h.evalCtx.SessionData().OptimizerUseVirtualComputedColumnStats = true
-	h.evalCtx.SessionData().OptimizerUseTrigramSimilarityOptimization = true
-	h.evalCtx.SessionData().OptimizerUseImprovedDistinctOnLimitHintCosting = true
-	h.evalCtx.SessionData().OptimizerUseImprovedTrigramSimilaritySelectivity = true
-	h.evalCtx.SessionData().TrigramSimilarityThreshold = 0.3
-	h.evalCtx.SessionData().OptimizerUseImprovedZigzagJoinCosting = true
-	h.evalCtx.SessionData().OptimizerUseImprovedMultiColumnSelectivityEstimate = true
-	h.evalCtx.SessionData().OptimizerProveImplicationWithVirtualComputedColumns = true
-	h.evalCtx.SessionData().OptimizerPushOffsetIntoIndexJoin = true
+	// Set session settings to their global defaults.
+	if err := sql.TestingResetSessionVariables(h.ctx, h.evalCtx); err != nil {
+		panic(errors.Wrap(err, "could not reset session variables"))
+	}
 
 	// Set up the test catalog.
 	h.testCat = testcat.New()

pkg/sql/opt/bench/testdata/slow-schemas.sql

@@ -869,21 +869,25 @@ CREATE TABLE seed64793 (
   _uuid UUID,
   _inet INET,
   _jsonb JSONB,
-  _enum greeting64793
+  _enum greeting64793,
+  INDEX (_int8, _float8, _date),
+  INVERTED INDEX (_jsonb)
 );
 
--- TODO(mgartner): Move this into the seed table definition.
-CREATE INDEX on seed64793 (_int8, _float8, _date);
-
--- TODO(mgartner): Move this into the seed table definition.
-CREATE INVERTED INDEX on seed64793 (_jsonb);
-
 -- Used in slow-query-2.
 CREATE TABLE table64793_2 (
-  col1_0 "char" NOT NULL, col1_1 OID NOT NULL, col1_2 BIT(38) NOT NULL,
-  col1_3 BIT(18) NOT NULL, col1_4 BYTES NOT NULL, col1_5 INT8 NOT NULL,
-  col1_6 INTERVAL NOT NULL, col1_7 BIT(33) NOT NULL, col1_8 INTERVAL NULL,
-  col1_9 GEOMETRY NOT NULL, col1_10 BOOL NOT NULL, col1_11 INT2,
+  col1_0 "char" NOT NULL,
+  col1_1 OID NOT NULL,
+  col1_2 BIT(38) NOT NULL,
+  col1_3 BIT(18) NOT NULL,
+  col1_4 BYTES NOT NULL,
+  col1_5 INT8 NOT NULL,
+  col1_6 INTERVAL NOT NULL,
+  col1_7 BIT(33) NOT NULL,
+  col1_8 INTERVAL NULL,
+  col1_9 GEOMETRY NOT NULL,
+  col1_10 BOOL NOT NULL,
+  col1_11 INT2,
   PRIMARY KEY (
     col1_4 ASC, col1_7 DESC, col1_1 ASC, col1_2 ASC, col1_10 ASC, col1_5,
     col1_0 ASC, col1_3, col1_6
@@ -896,37 +900,45 @@ CREATE TABLE table64793_2 (
 
 -- Used in slow-query-2.
 CREATE TABLE table64793_3 (
-  col2_0 NAME NOT NULL, col2_1 TIMETZ NOT NULL,
-  PRIMARY KEY (col2_0 ASC, col2_1),
+  col2_0 NAME NOT NULL,
+  col2_1 TIMETZ NOT NULL,
   col2_2 STRING NOT NULL AS (lower(col2_0)) VIRTUAL,
-  UNIQUE (col2_0 DESC, col2_2 DESC, col2_1)
-  WHERE (table64793_3.col2_2 > e'\U00002603':::STRING)
-  OR (table64793_3.col2_0 != '"':::STRING),
+  PRIMARY KEY (col2_0 ASC, col2_1),
+  UNIQUE (col2_0 DESC, col2_2 DESC, col2_1) WHERE (table64793_3.col2_2 > e'\U00002603':::STRING) OR (table64793_3.col2_0 != '"':::STRING),
   UNIQUE (col2_1 ASC, col2_2, col2_0),
-  UNIQUE (col2_0 DESC,col2_1, col2_2),
+  UNIQUE (col2_0 DESC, col2_1, col2_2),
   INDEX (col2_1 DESC),
-  UNIQUE (col2_2 DESC, col2_0 ASC)
-  WHERE table64793_3.col2_2 = '"':::STRING
+  UNIQUE (col2_2 DESC, col2_0 ASC) WHERE table64793_3.col2_2 = '"':::STRING
 );
 
 -- Used in slow-query-2.
 CREATE TABLE table64793_4 (
-  col2_0 NAME NOT NULL, col2_1 TIMETZ NOT NULL, col3_2 REGPROC NOT NULL,
-  col3_3 "char", col3_4 BOX2D, col3_5 INT8 NULL, col3_6 TIMESTAMP NOT NULL,
-  col3_7 FLOAT8, col3_8 INT4 NULL, col3_9 INET NULL, col3_10 UUID NOT NULL,
-  col3_11 UUID NULL, col3_12 INT2 NOT NULL, col3_13 BIT(34),
-  col3_14 REGPROCEDURE NULL, col3_15 FLOAT8 NULL,
+  col2_0 NAME NOT NULL,
+  col2_1 TIMETZ NOT NULL,
+  col3_2 REGPROC NOT NULL,
+  col3_3 "char",
+  col3_4 BOX2D,
+  col3_5 INT8 NULL,
+  col3_6 TIMESTAMP NOT NULL,
+  col3_7 FLOAT8,
+  col3_8 INT4 NULL,
+  col3_9 INET NULL,
+  col3_10 UUID NOT NULL,
+  col3_11 UUID NULL,
+  col3_12 INT2 NOT NULL,
+  col3_13 BIT(34),
+  col3_14 REGPROCEDURE NULL,
+  col3_15 FLOAT8 NULL,
   PRIMARY KEY (
     col2_0 ASC, col2_1, col3_11 DESC, col3_13, col3_6, col3_3 DESC,
     col3_15 ASC, col3_2 ASC, col3_4 ASC, col3_9 DESC, col3_12 ASC,
     col3_8 ASC, col3_5, col3_14 ASC
   ),
-  UNIQUE (col3_2, col3_8 ASC)
-  WHERE ((((table64793_4.col3_5 < 0:::INT8)
-  AND (table64793_4.col3_3 != '':::STRING))
-  AND (table64793_4.col2_1 < '00:00:00+15:59:00':::TIMETZ))
-  AND (table64793_4.col3_12 > 0:::INT8))
-  AND (table64793_4.col3_15 <= 1.7976931348623157e+308:::FLOAT8),
+  UNIQUE (col3_2, col3_8 ASC) WHERE ((((col3_5 < 0:::INT8)
+    AND (col3_3 != '':::STRING))
+    AND (col2_1 < '00:00:00+15:59:00':::TIMETZ))
+    AND (col3_12 > 0:::INT8))
+    AND (col3_15 <= 1.7976931348623157e+308:::FLOAT8),
   UNIQUE (col3_10 DESC, col3_3 ASC, col2_1 DESC, col3_9 ASC)
 );
 

pkg/sql/opt/exec/explain/testdata/gists

@@ -647,6 +647,7 @@ explain(shape):
     └── • scan
           table: abc@abc_pkey
           spans: 1+ spans
+          locking strength: for update
 explain(gist):
 • update
 │ table: abc
@@ -679,6 +680,7 @@ explain(shape):
     └── • scan
           table: abc@abc_pkey
           spans: 1+ spans
+          locking strength: for update
 explain(gist):
 • upsert
 │ into: abc()

pkg/sql/opt/exec/explain/testdata/gists_tpce

@@ -237,6 +237,7 @@ explain(shape):
 │                   └── • scan
 │                         table: last_trade@last_trade_pkey
 │                         spans: 1+ spans
+│                         locking strength: for update
 │
 ├── • subquery
 │   │ id: @S2

pkg/sql/opt/testutils/opttester/BUILD.bazel

@@ -26,7 +26,6 @@ go_library(
         "//pkg/sql",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/schemaexpr",
-        "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/opt",
         "//pkg/sql/opt/cat",
         "//pkg/sql/opt/exec",

pkg/sql/opt/testutils/opttester/opt_tester.go

@@ -42,7 +42,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec"
@@ -271,57 +270,33 @@ type Flags struct {
 
 // New constructs a new instance of the OptTester for the given SQL statement.
 // Metadata used by the SQL query is accessed via the catalog.
-func New(catalog cat.Catalog, sql string) *OptTester {
+func New(catalog cat.Catalog, sqlStr string) *OptTester {
 	ctx := context.Background()
 	ot := &OptTester{
 		catalog: catalog,
-		sql:     sql,
+		sql:     sqlStr,
 		ctx:     ctx,
 		semaCtx: tree.MakeSemaContext(catalog),
 		evalCtx: eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings()),
 	}
 	ot.f = &norm.Factory{}
 	ot.f.Init(ot.ctx, &ot.evalCtx, ot.catalog)
-	ot.evalCtx.SessionData().ReorderJoinsLimit = opt.DefaultJoinOrderLimit
-	ot.evalCtx.SessionData().OptimizerUseMultiColStats = true
 	ot.Flags.ctx = ot.ctx
 	ot.Flags.evalCtx = ot.evalCtx
 	ot.semaCtx.SearchPath = tree.EmptySearchPath
 	// To allow opttester tests to use now(), we hardcode a preset transaction
 	// time. May 10, 2017 is a historic day: the release date of CockroachDB 1.0.
 	ot.evalCtx.TxnTimestamp = time.Date(2017, 05, 10, 13, 0, 0, 0, time.UTC)
 
-	// Set any OptTester-wide session flags here.
+	// Set session settings to their global defaults.
+	if err := sql.TestingResetSessionVariables(ctx, ot.evalCtx); err != nil {
+		panic(errors.Wrap(err, "could not reset session variables"))
+	}
 
+	// Set non-default session settings.
 	ot.evalCtx.SessionData().UserProto = username.MakeSQLUsernameFromPreNormalizedString("opttester").EncodeProto()
 	ot.evalCtx.SessionData().Database = "defaultdb"
 	ot.evalCtx.SessionData().ZigzagJoinEnabled = true
-	ot.evalCtx.SessionData().OptimizerUseForecasts = true
-	ot.evalCtx.SessionData().OptimizerUseHistograms = true
-	ot.evalCtx.SessionData().LocalityOptimizedSearch = true
-	ot.evalCtx.SessionData().ReorderJoinsLimit = opt.DefaultJoinOrderLimit
-	ot.evalCtx.SessionData().InsertFastPath = true
-	ot.evalCtx.SessionData().OptSplitScanLimit = tabledesc.MaxBucketAllowed
-	ot.evalCtx.SessionData().VariableInequalityLookupJoinEnabled = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedDisjunctionStats = true
-	ot.evalCtx.SessionData().OptimizerUseLimitOrderingForStreamingGroupBy = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedSplitDisjunctionForJoins = true
-	ot.evalCtx.SessionData().OptimizerAlwaysUseHistograms = true
-	ot.evalCtx.SessionData().OptimizerHoistUncorrelatedEqualitySubqueries = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedComputedColumnFiltersDerivation = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedJoinElimination = true
-	ot.evalCtx.SessionData().OptimizerUseProvidedOrderingFix = true
-	ot.evalCtx.SessionData().OptimizerMergeJoinsEnabled = true
-	ot.evalCtx.SessionData().OptimizerUseVirtualComputedColumnStats = true
-	ot.evalCtx.SessionData().OptimizerUseTrigramSimilarityOptimization = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedDistinctOnLimitHintCosting = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedTrigramSimilaritySelectivity = true
-	ot.evalCtx.SessionData().TrigramSimilarityThreshold = 0.3
-	ot.evalCtx.SessionData().OptimizerUseImprovedZigzagJoinCosting = true
-	ot.evalCtx.SessionData().OptimizerUseImprovedMultiColumnSelectivityEstimate = true
-	ot.evalCtx.SessionData().OptimizerProveImplicationWithVirtualComputedColumns = true
-	ot.evalCtx.SessionData().OptimizerPushOffsetIntoIndexJoin = true
-	ot.evalCtx.SessionData().OptimizerUsePolymorphicParameterFix = true
 
 	return ot
 }
@@ -979,7 +954,7 @@ func (f *Flags) Set(arg datadriven.CmdArg) error {
 			if len(s) != 2 {
 				return errors.Errorf("Expected both session variable name and value for set flag")
 			}
-			err := sql.SetSessionVariable(f.ctx, f.evalCtx, s[0], s[1])
+			err := sql.TestingSetSessionVariable(f.ctx, f.evalCtx, s[0], s[1])
 			if err != nil {
 				return err
 			}

pkg/sql/vars.go

@@ -3534,19 +3534,18 @@ func init() {
 	}()
 }
 
-// SetSessionVariable sets a new value for session setting `varName` in the
+// TestingSetSessionVariable sets a new value for session setting `varName` in the
 // session settings owned by `evalCtx`, returning an error if not successful.
 // This function should only be used for testing. For general-purpose code,
 // please use SessionAccessor.SetSessionVar instead.
-func SetSessionVariable(
+func TestingSetSessionVariable(
 	ctx context.Context, evalCtx eval.Context, varName, varValue string,
 ) (err error) {
 	err = CheckSessionVariableValueValid(ctx, evalCtx.Settings, varName, varValue)
 	if err != nil {
 		return err
 	}
 	sdMutatorBase := sessionDataMutatorBase{
-		defaults: make(map[string]string),
 		settings: evalCtx.Settings,
 	}
 	sdMutator := sessionDataMutator{
@@ -3562,6 +3561,28 @@ func SetSessionVariable(
 	return sVar.Set(ctx, sdMutator, varValue)
 }
 
+// TestingResetSessionVariables resets all session settings in evalCtx to their
+// global default, if they have a global default.
+func TestingResetSessionVariables(ctx context.Context, evalCtx eval.Context) (err error) {
+	sdMutatorBase := sessionDataMutatorBase{
+		settings: evalCtx.Settings,
+	}
+	sdMutator := sessionDataMutator{
+		data:                        evalCtx.SessionData(),
+		sessionDataMutatorBase:      sdMutatorBase,
+		sessionDataMutatorCallbacks: sessionDataMutatorCallbacks{},
+	}
+	for _, v := range varGen {
+		if v.Set == nil || v.GlobalDefault == nil {
+			continue
+		}
+		if err := v.Set(ctx, sdMutator, v.GlobalDefault(&evalCtx.Settings.SV)); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // makePostgresBoolGetStringValFn returns a function that evaluates and returns
 // a string representation of the first argument value.
 func makePostgresBoolGetStringValFn(varName string) getStringValFn {