sql: optimizer always on

This change deprecates the `optimizer` session setting and associated
cluster setting. It is no longer possible to disable the optimizer.
The settings still exist but the only allowed value is "on" (mostly so
that `SET OPTIMIZER = ON` still works).

The logic test configurations are reworked: they now all use the
optimizer, and the `-opt` versions are gone (e.g. what used to be
`local-opt` is now `local`). The HP-specific planner tests are
removed.

Release note: None

Makefile

@@ -51,10 +51,10 @@ TESTS := .
 ## Benchmarks to run for use with `make bench`.
 BENCHES :=
 
-## Space delimited list of logic test files to run, for make testlogic/testccllogic/testoptlogic/testplannerlogic.
+## Space delimited list of logic test files to run, for make testlogic/testccllogic/testoptlogic.
 FILES :=
 
-## Name of a logic test configuration to run, for make testlogic/testccllogic/testoptlogic/testplannerlogic.
+## Name of a logic test configuration to run, for make testlogic/testccllogic/testoptlogic.
 ## (default: all configs. It's not possible yet to specify multiple configs in this way.)
 TESTCONFIG :=
 
@@ -947,7 +947,7 @@ bench benchshort: TESTTIMEOUT := $(BENCHTIMEOUT)
 # that longer running benchmarks can skip themselves.
 benchshort: override TESTFLAGS += -benchtime=1ns -short
 
-.PHONY: check test testshort testrace testlogic testbaselogic testplannerlogic testccllogic testoptlogic bench benchshort
+.PHONY: check test testshort testrace testlogic testbaselogic testccllogic testoptlogic bench benchshort
 test: ## Run tests.
 check test testshort testrace bench benchshort:
 	$(xgo) test $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" $(if $(BENCHES),-bench "$(BENCHES)") -timeout $(TESTTIMEOUT) $(PKG) $(TESTFLAGS)
@@ -970,14 +970,11 @@ roachprod-stress roachprod-stressrace: bin/roachprod-stress
 	    -test.run "$(TESTS)" $(filter-out -v,$(TESTFLAGS)) -test.v -test.timeout $(TESTTIMEOUT); \
 	fi
 
-testlogic: testbaselogic testplannerlogic testoptlogic testccllogic
+testlogic: testbaselogic testoptlogic testccllogic
 
 testbaselogic: ## Run SQL Logic Tests.
 testbaselogic: bin/logictest
 
-testplannerlogic: ## Run SQL Logic Tests for the heuristic planner.
-testplannerlogic: bin/logictest
-
 testccllogic: ## Run SQL CCL Logic Tests.
 testccllogic: bin/logictestccl
 
@@ -987,7 +984,6 @@ testoptlogic: bin/logictestopt
 logic-test-selector := $(if $(TESTCONFIG),^$(TESTCONFIG)$$)/$(if $(FILES),^$(subst $(space),$$|^,$(FILES))$$)/$(SUBTESTS)
 testbaselogic: TESTS := TestLogic/$(logic-test-selector)
 testccllogic: TESTS := TestCCLLogic/$(logic-test-selector)
-testplannerlogic: TESTS := TestPlannerLogic/$(logic-test-selector)
 testoptlogic: TESTS := TestExecBuild/$(logic-test-selector)
 
 # Note: we specify -config here in addition to the filter on TESTS
@@ -996,8 +992,8 @@ testoptlogic: TESTS := TestExecBuild/$(logic-test-selector)
 # does not prevent loading and initializing every default config in
 # turn (including setting up the test clusters, etc.). By specifying
 # -config, the extra initialization overhead is averted.
-testbaselogic testccllogic testplannerlogic testoptlogic: TESTFLAGS := -test.v $(if $(FILES),-show-sql) $(if $(TESTCONFIG),-config $(TESTCONFIG))
-testbaselogic testccllogic testplannerlogic testoptlogic:
+testbaselogic testccllogic testoptlogic: TESTFLAGS := -test.v $(if $(FILES),-show-sql) $(if $(TESTCONFIG),-config $(TESTCONFIG))
+testbaselogic testccllogic testoptlogic:
 	cd $($(<F)-package) && $(<F) -test.run "$(TESTS)" -test.timeout $(TESTTIMEOUT) $(TESTFLAGS)
 
 testraceslow: override GOFLAGS += -race

build/teamcity-sqllogictest.sh

@@ -23,6 +23,6 @@ build/builder.sh \
 # numeric typing differences.
 build/builder.sh \
   stdbuf -oL -eL \
-  make test TESTFLAGS="-v -bigtest -config local-opt,fakedist-opt -flex-types" TESTTIMEOUT='24h' PKG='./pkg/sql/logictest' TESTS='^TestSqlLiteCorrelatedLogic$$' 2>&1 \
+  make test TESTFLAGS="-v -bigtest -config local,fakedist -flex-types" TESTTIMEOUT='24h' PKG='./pkg/sql/logictest' TESTS='^TestSqlLiteCorrelatedLogic$$' 2>&1 \
   | tee "artifacts/sqllite-correlated.log" \
   | go-test-teamcity

build/teamcity-testlogicrace.sh

@@ -25,7 +25,7 @@ build/builder.sh \
 	stdbuf -oL -eL \
 	make testrace \
 	PKG=./pkg/sql/logictest \
-	TESTS='^TestLogic/local-opt$$' \
+	TESTS='^TestLogic/local$$' \
 	TESTFLAGS='-optimizer-cost-perturbation=0.9 -v' \
 	ENABLE_ROCKSDB_ASSERTIONS=1 \
 	2>&1 \
@@ -48,7 +48,7 @@ for file in $LOGICTESTS; do
 	        stdbuf -oL -eL \
 	        make testrace \
 	        PKG=./pkg/sql/logictest \
-	        TESTS='^TestLogic/local-opt/'${file}'$$' \
+	        TESTS='^TestLogic/local/'${file}'$$' \
 	        TESTFLAGS='-disable-opt-rule-probability=0.5 -v' \
 	        ENABLE_ROCKSDB_ASSERTIONS=1 \
 	        2>&1 \

docs/generated/settings/settings.html

@@ -84,7 +84,7 @@
 <tr><td><code>server.web_session_timeout</code></td><td>duration</td><td><code>168h0m0s</code></td><td>the duration that a newly created web session will be valid</td></tr>
 <tr><td><code>sql.defaults.default_int_size</code></td><td>integer</td><td><code>8</code></td><td>the size, in bytes, of an INT type</td></tr>
 <tr><td><code>sql.defaults.distsql</code></td><td>enumeration</td><td><code>auto</code></td><td>default distributed SQL execution mode [off = 0, auto = 1, on = 2]</td></tr>
-<tr><td><code>sql.defaults.optimizer</code></td><td>enumeration</td><td><code>on</code></td><td>default cost-based optimizer mode [off = 0, on = 1, local = 2]</td></tr>
+<tr><td><code>sql.defaults.optimizer</code></td><td>enumeration</td><td><code>on</code></td><td>default cost-based optimizer mode [on = 1]</td></tr>
 <tr><td><code>sql.defaults.reorder_joins_limit</code></td><td>integer</td><td><code>4</code></td><td>default number of joins to reorder</td></tr>
 <tr><td><code>sql.defaults.results_buffer.size</code></td><td>byte size</td><td><code>16 KiB</code></td><td>default size of the buffer that accumulates results for a statement or a batch of statements before they are sent to the client. This can be overridden on an individual connection with the 'results_buffer_size' parameter. Note that auto-retries generally only happen while no results have been delivered to the client, so reducing this size can increase the number of retriable errors a client receives. On the other hand, increasing the buffer size can increase the delay until the client receives the first result row. Updating the setting only affects new connections. Setting to 0 disables any buffering.</td></tr>
 <tr><td><code>sql.defaults.serial_normalization</code></td><td>enumeration</td><td><code>rowid</code></td><td>default handling of SERIAL in table definitions [rowid = 0, virtual_sequence = 1, sql_sequence = 2]</td></tr>

pkg/ccl/logictestccl/testdata/logic_test/as_of

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 statement ok
 CREATE TABLE t (i INT)

pkg/ccl/logictestccl/testdata/logic_test/case_sensitive_names

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 statement ok
 CREATE TABLE p (a INT PRIMARY KEY) PARTITION BY LIST (a) (

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 query IITTITTT colnames
 SELECT * FROM crdb_internal.partitions

pkg/ccl/logictestccl/testdata/logic_test/distsql_partitioning

@@ -1,4 +1,4 @@
-# LogicTest: 5node-dist-opt
+# LogicTest: 5node-dist
 
 # Tests for the show partitions command.
 

pkg/ccl/logictestccl/testdata/logic_test/drop_index

@@ -1,4 +1,4 @@
-# LogicTest: 5node-dist-opt
+# LogicTest: 5node-dist
 
 statement ok
 CREATE TABLE t (

pkg/ccl/logictestccl/testdata/logic_test/partitioning

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 statement error syntax
 CREATE TABLE t (a INT, b INT, c INT, PRIMARY KEY (a, b)) PARTITION BY LIST ()

pkg/ccl/logictestccl/testdata/logic_test/partitioning_constrained_scans

@@ -1,4 +1,4 @@
-# LogicTest: local-opt fakedist-opt
+# LogicTest: local fakedist
 
 # Simple partitioning example.
 statement ok

pkg/ccl/logictestccl/testdata/logic_test/partitioning_index

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 statement ok
 CREATE TABLE ok1 (

pkg/ccl/logictestccl/testdata/logic_test/restore

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 # Check that we get through parsing and license check.
 statement error pq: failed to read backup descriptor: unsupported storage scheme: ""

pkg/ccl/logictestccl/testdata/logic_test/role

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local
 
 query T colnames
 SHOW ROLES

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -1,4 +1,4 @@
-# LogicTest: 5node-dist-opt
+# LogicTest: 5node-dist
 
 # Ensure that cost-based-optimizer uses an index with zone constraints that most
 # closely matches the gateway's locality. Use "retry" option, since it can take

pkg/sql/conn_executor_exec.go

@@ -24,7 +24,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
@@ -659,13 +658,9 @@ func (ex *connExecutor) dispatchToExecutionEngine(
 
 	ex.sessionTracing.TracePlanCheckStart(ctx)
 	distributePlan := false
-	// If we use the optimizer and we are in "local" mode, don't try to
-	// distribute.
-	if ex.sessionData.OptimizerMode != sessiondata.OptimizerLocal {
-		planner.prepareForDistSQLSupportCheck()
-		distributePlan = shouldDistributePlan(
-			ctx, ex.sessionData.DistSQLMode, ex.server.cfg.DistSQLPlanner, planner.curPlan.plan)
-	}
+	planner.prepareForDistSQLSupportCheck()
+	distributePlan = shouldDistributePlan(
+		ctx, ex.sessionData.DistSQLMode, ex.server.cfg.DistSQLPlanner, planner.curPlan.plan)
 	ex.sessionTracing.TracePlanCheckEnd(ctx, nil, distributePlan)
 
 	if ex.server.cfg.TestingKnobs.BeforeExecute != nil {
@@ -727,25 +722,16 @@ func (ex *connExecutor) makeExecPlan(ctx context.Context, planner *planner) erro
 	// in error cases.
 	planner.curPlan = planTop{AST: stmt.AST}
 
-	if optMode := ex.sessionData.OptimizerMode; optMode != sessiondata.OptimizerOff {
-		log.VEvent(ctx, 2, "generating optimizer plan")
-		var result *planTop
-		var err error
-		result, err = planner.makeOptimizerPlan(ctx)
-		if err != nil {
-			log.VEventf(ctx, 1, "optimizer plan failed: %v", err)
-			return err
-		}
-		planner.curPlan = *result
-		return nil
+	log.VEvent(ctx, 2, "generating optimizer plan")
+	var result *planTop
+	var err error
+	result, err = planner.makeOptimizerPlan(ctx)
+	if err != nil {
+		log.VEventf(ctx, 1, "optimizer plan failed: %v", err)
+		return err
 	}
-
-	log.VEvent(ctx, 2, "optimizer disabled")
-	// Use the heuristic planner.
-	optFlags := planner.curPlan.flags
-	err := planner.makePlan(ctx)
-	planner.curPlan.flags |= optFlags
-	return err
+	planner.curPlan = *result
+	return nil
 }
 
 // saveLogicalPlanDescription returns whether we should save this as a sample logical plan

pkg/sql/conn_executor_prepare.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/util/fsm"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -182,8 +181,6 @@ func (ex *connExecutor) populatePrepared(
 	if err := p.semaCtx.Placeholders.Init(stmt.NumPlaceholders, placeholderHints); err != nil {
 		return 0, err
 	}
-	prepared := stmt.Prepared
-
 	p.extendedEvalCtx.PrepareOnly = true
 
 	protoTS, err := p.isAsOf(stmt.AST)
@@ -203,52 +200,14 @@ func (ex *connExecutor) populatePrepared(
 	// As of right now, the optimizer only works on SELECT statements and will
 	// fallback for all others, so this should be safe for the foreseeable
 	// future.
-	var flags planFlags
-	if optMode := ex.sessionData.OptimizerMode; optMode != sessiondata.OptimizerOff {
-		log.VEvent(ctx, 2, "preparing using optimizer")
-		var err error
-		flags, err = p.prepareUsingOptimizer(ctx)
-		if err == nil {
-			log.VEvent(ctx, 2, "optimizer prepare succeeded")
-			// stmt.Prepared fields have been populated.
-			return flags, nil
-		}
+	flags, err := p.prepareUsingOptimizer(ctx)
+	if err != nil {
 		log.VEventf(ctx, 1, "optimizer prepare failed: %v", err)
 		return 0, err
 	}
-	log.VEvent(ctx, 2, "optimizer disabled (prepare)")
-
-	// Fallback on the heuristic planner if the optimizer was not enabled: create
-	// a plan for the statement to figure out the typing, then close the plan.
-	prepared.AnonymizedStr = anonymizeStmt(stmt.AST)
-	if err := p.prepare(ctx, stmt.AST); err != nil {
-		return 0, err
-	}
-
-	if p.curPlan.plan == nil {
-		// Statement with no result columns and no support for placeholders.
-		//
-		// Note: we're combining `flags` which comes from
-		// `prepareUsingOptimizer`, with `p.curPlan.flags` which ensures
-		// the new flags combine with the existing flags (this is used
-		// e.g. to maintain the count of times the optimizer was used).
-		return flags | p.curPlan.flags, nil
-	}
-	defer p.curPlan.close(ctx)
-
-	prepared.Columns = p.curPlan.columns()
-	for _, c := range prepared.Columns {
-		if err := checkResultType(c.Typ); err != nil {
-			return 0, err
-		}
-	}
-	// Verify that all placeholder types have been set.
-	if err := p.semaCtx.Placeholders.Types.AssertAllSet(); err != nil {
-		return 0, err
-	}
-	prepared.Types = p.semaCtx.Placeholders.Types
-	// The flags are combined, see the comment above for why.
-	return flags | p.curPlan.flags, nil
+	log.VEvent(ctx, 2, "optimizer prepare succeeded")
+	// stmt.Prepared fields have been populated.
+	return flags, nil
 }
 
 func (ex *connExecutor) execBind(

pkg/sql/exec_util.go

@@ -115,15 +115,14 @@ var traceSessionEventLogEnabled = settings.RegisterBoolSetting(
 	"set to true to enable session tracing", false,
 )
 
-// OptimizerClusterMode controls the cluster default for when the cost-based optimizer is used.
-var OptimizerClusterMode = settings.RegisterEnumSetting(
+// This used to control the cluster default for when the cost-based optimizer is used.
+// It is deprecated; the only allowable setting is "on".
+var _ = settings.RegisterEnumSetting(
 	"sql.defaults.optimizer",
 	"default cost-based optimizer mode",
 	"on",
 	map[int64]string{
-		int64(sessiondata.OptimizerLocal): "local",
-		int64(sessiondata.OptimizerOff):   "off",
-		int64(sessiondata.OptimizerOn):    "on",
+		1: "on",
 	},
 )
 
@@ -1824,10 +1823,6 @@ func (m *sessionDataMutator) SetVectorizeRowCountThreshold(val uint64) {
 	m.data.VectorizeRowCountThreshold = val
 }
 
-func (m *sessionDataMutator) SetOptimizerMode(val sessiondata.OptimizerMode) {
-	m.data.OptimizerMode = val
-}
-
 func (m *sessionDataMutator) SetOptimizerFKs(val bool) {
 	m.data.OptimizerFKs = val
 }