logictest: clean up disabling of auto stats

pkg/sql/logictest/logic.go

@@ -484,8 +484,6 @@ type testClusterConfig struct {
 	overrideDistSQLMode string
 	// if non-empty, overrides the default vectorize mode.
 	overrideVectorize string
-	// if non-empty, overrides the default automatic statistics mode.
-	overrideAutoStats string
 	// if non-empty, overrides the default experimental DistSQL planning mode.
 	overrideExperimentalDistSQLPlanning string
 	// if set, queries using distSQL processors or vectorized operators that can
@@ -680,20 +678,17 @@ var logicTestConfigs = []testClusterConfig{
 		name:                "local",
 		numNodes:            1,
 		overrideDistSQLMode: "off",
-		overrideAutoStats:   "false",
 	},
 	{
 		name:                "local-vec-off",
 		numNodes:            1,
 		overrideDistSQLMode: "off",
-		overrideAutoStats:   "false",
 		overrideVectorize:   "off",
 	},
 	{
 		name:                "[email protected]",
 		numNodes:            1,
 		overrideDistSQLMode: "off",
-		overrideAutoStats:   "false",
 		bootstrapVersion:    roachpb.Version{Major: 1},
 		binaryVersion:       roachpb.Version{Major: 1, Minor: 1},
 		disableUpgrade:      true,
@@ -702,30 +697,26 @@ var logicTestConfigs = []testClusterConfig{
 		name:                                "local-spec-planning",
 		numNodes:                            1,
 		overrideDistSQLMode:                 "off",
-		overrideAutoStats:                   "false",
 		overrideExperimentalDistSQLPlanning: "on",
 	},
 	{
 		name:                "fakedist",
 		numNodes:            3,
 		useFakeSpanResolver: true,
 		overrideDistSQLMode: "on",
-		overrideAutoStats:   "false",
 	},
 	{
 		name:                "fakedist-vec-off",
 		numNodes:            3,
 		useFakeSpanResolver: true,
 		overrideDistSQLMode: "on",
-		overrideAutoStats:   "false",
 		overrideVectorize:   "off",
 	},
 	{
 		name:                       "fakedist-metadata",
 		numNodes:                   3,
 		useFakeSpanResolver:        true,
 		overrideDistSQLMode:        "on",
-		overrideAutoStats:          "false",
 		distSQLMetadataTestEnabled: true,
 		skipShort:                  true,
 	},
@@ -734,7 +725,6 @@ var logicTestConfigs = []testClusterConfig{
 		numNodes:            3,
 		useFakeSpanResolver: true,
 		overrideDistSQLMode: "on",
-		overrideAutoStats:   "false",
 		sqlExecUseDisk:      true,
 		skipShort:           true,
 	},
@@ -743,36 +733,31 @@ var logicTestConfigs = []testClusterConfig{
 		numNodes:                            3,
 		useFakeSpanResolver:                 true,
 		overrideDistSQLMode:                 "on",
-		overrideAutoStats:                   "false",
 		overrideExperimentalDistSQLPlanning: "on",
 	},
 	{
 		name:                "5node",
 		numNodes:            5,
 		overrideDistSQLMode: "on",
-		overrideAutoStats:   "false",
 	},
 	{
 		name:                       "5node-metadata",
 		numNodes:                   5,
 		overrideDistSQLMode:        "on",
-		overrideAutoStats:          "false",
 		distSQLMetadataTestEnabled: true,
 		skipShort:                  true,
 	},
 	{
 		name:                "5node-disk",
 		numNodes:            5,
 		overrideDistSQLMode: "on",
-		overrideAutoStats:   "false",
 		sqlExecUseDisk:      true,
 		skipShort:           true,
 	},
 	{
 		name:                                "5node-spec-planning",
 		numNodes:                            5,
 		overrideDistSQLMode:                 "on",
-		overrideAutoStats:                   "false",
 		overrideExperimentalDistSQLPlanning: "on",
 	},
 	{
@@ -781,23 +766,19 @@ var logicTestConfigs = []testClusterConfig{
 		// logictest command.
 		// To run a logic test with this config as a directive, run:
 		// make test PKG=./pkg/ccl/logictestccl TESTS=TestTenantLogic//<test_name>
-		name:     threeNodeTenantConfigName,
-		numNodes: 3,
-		// overrideAutoStats will disable automatic stats on the cluster this tenant
-		// is connected to.
-		overrideAutoStats: "false",
-		useTenant:         true,
-		isCCLConfig:       true,
+		name:        threeNodeTenantConfigName,
+		numNodes:    3,
+		useTenant:   true,
+		isCCLConfig: true,
 	},
 	// Regions and zones below are named deliberately, and contain "-"'s to be reflective
 	// of the naming convention in public clouds.  "-"'s are handled differently in SQL
 	// (they're double double quoted) so we explicitly test them here to ensure that
 	// the multi-region code handles them correctly.
 
 	{
-		name:              "multiregion-invalid-locality",
-		numNodes:          3,
-		overrideAutoStats: "false",
+		name:     "multiregion-invalid-locality",
+		numNodes: 3,
 		localities: map[int]roachpb.Locality{
 			1: {
 				Tiers: []roachpb.Tier{
@@ -817,9 +798,8 @@ var logicTestConfigs = []testClusterConfig{
 		},
 	},
 	{
-		name:              "multiregion-3node-3superlongregions",
-		numNodes:          3,
-		overrideAutoStats: "false",
+		name:     "multiregion-3node-3superlongregions",
+		numNodes: 3,
 		localities: map[int]roachpb.Locality{
 			1: {
 				Tiers: []roachpb.Tier{
@@ -839,36 +819,31 @@ var logicTestConfigs = []testClusterConfig{
 		},
 	},
 	{
-		name:              "multiregion-9node-3region-3azs",
-		numNodes:          9,
-		overrideAutoStats: "false",
-		localities:        multiregion9node3region3azsLocalities,
+		name:       "multiregion-9node-3region-3azs",
+		numNodes:   9,
+		localities: multiregion9node3region3azsLocalities,
 	},
 	{
-		name:              "multiregion-9node-3region-3azs-tenant",
-		numNodes:          9,
-		overrideAutoStats: "false",
-		localities:        multiregion9node3region3azsLocalities,
-		useTenant:         true,
+		name:       "multiregion-9node-3region-3azs-tenant",
+		numNodes:   9,
+		localities: multiregion9node3region3azsLocalities,
+		useTenant:  true,
 	},
 	{
 		name:              "multiregion-9node-3region-3azs-vec-off",
 		numNodes:          9,
-		overrideAutoStats: "false",
 		localities:        multiregion9node3region3azsLocalities,
 		overrideVectorize: "off",
 	},
 	{
-		name:              "multiregion-15node-5region-3azs",
-		numNodes:          15,
-		overrideAutoStats: "false",
-		localities:        multiregion15node5region3azsLocalities,
+		name:       "multiregion-15node-5region-3azs",
+		numNodes:   15,
+		localities: multiregion15node5region3azsLocalities,
 	},
 	{
 		name:                "local-mixed-21.2-22.1",
 		numNodes:            1,
 		overrideDistSQLMode: "off",
-		overrideAutoStats:   "false",
 		bootstrapVersion:    roachpb.Version{Major: 21, Minor: 2},
 		binaryVersion:       roachpb.Version{Major: 22, Minor: 1},
 		disableUpgrade:      true,
@@ -1752,32 +1727,28 @@ func (t *logicTest) newCluster(serverArgs TestServerArgs, opts []clusterOpt) {
 			}
 		}
 
-		if cfg.overrideAutoStats != "" {
-			if _, err := conn.Exec(
-				"SET CLUSTER SETTING sql.stats.automatic_collection.enabled = $1::bool", cfg.overrideAutoStats,
-			); err != nil {
-				t.Fatal(err)
-			}
-		} else {
-			// Background stats collection is enabled by default, but we've seen tests
-			// flake with it on. When the issue manifests, it seems to be around a
-			// schema change transaction getting pushed, which causes it to increment a
-			// table ID twice instead of once, causing non-determinism.
-			//
-			// In the short term, we disable auto stats by default to avoid the flakes.
-			//
-			// In the long run, these tests should be running with default settings as
-			// much as possible, so we likely want to address this. Two options are
-			// either making schema changes more resilient to being pushed or possibly
-			// making auto stats avoid pushing schema change transactions. There might
-			// be other better alternatives than these.
-			//
-			// See #37751 for details.
-			if _, err := conn.Exec(
-				"SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false",
-			); err != nil {
-				t.Fatal(err)
-			}
+		// We disable the automatic stats collection in order to have
+		// deterministic tests.
+		//
+		// We've also seen tests flake with it on. When the issue manifests, it
+		// seems to be around a schema change transaction getting pushed, which
+		// causes it to increment a table ID twice instead of once, causing
+		// non-determinism.
+		//
+		// In the short term, we disable auto stats by default to avoid the
+		// flakes.
+		//
+		// In the long run, these tests should be running with default settings
+		// as much as possible, so we likely want to address this. Two options
+		// are either making schema changes more resilient to being pushed or
+		// possibly making auto stats avoid pushing schema change transactions.
+		// There might be other better alternatives than these.
+		//
+		// See #37751 for details.
+		if _, err := conn.Exec(
+			"SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false",
+		); err != nil {
+			t.Fatal(err)
 		}
 
 		if cfg.overrideExperimentalDistSQLPlanning != "" {

pkg/sql/logictest/testdata/logic_test/delete

@@ -262,10 +262,6 @@ COMMIT
 
 subtest regression_33361
 
-# Disable automatic stats to avoid flakiness (sometimes causes retry errors).
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 statement ok
 CREATE TABLE t33361(x INT PRIMARY KEY, y INT UNIQUE, z INT); INSERT INTO t33361 VALUES (1, 2, 3)
 

pkg/sql/logictest/testdata/logic_test/distsql_event_log

@@ -2,9 +2,6 @@
 # CREATE STATISTICS
 ###################
 
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 # This test verifies that events are posted for table statistics creation.
 statement ok
 SET CLUSTER SETTING sql.stats.post_events.enabled = TRUE

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -1,9 +1,6 @@
 # LogicTest: 5node 5node-metadata
 
-# Disable automatic stats.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
+# Disable histogram collection.
 statement ok
 SET CLUSTER SETTING sql.stats.histogram_collection.enabled = false
 

pkg/sql/logictest/testdata/logic_test/index_join

@@ -5,7 +5,6 @@
 # tests.
 
 statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false;
 CREATE TABLE lineitem
 (
     l_orderkey int PRIMARY KEY,

pkg/sql/logictest/testdata/logic_test/inv_stats

@@ -3,10 +3,6 @@
 # Tests that verify we retrieve the stats correctly. Note that we can't create
 # statistics if distsql mode is OFF.
 
-# Disable automatic stats to prevent flakes if auto stats run.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 statement ok
 CREATE TABLE t (j JSON, g GEOMETRY);
 CREATE INVERTED INDEX ON t (j);

pkg/sql/logictest/testdata/logic_test/partial_index

@@ -1036,7 +1036,6 @@ DELETE from u
 subtest join
 
 statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false;
 CREATE TABLE join_small (m INT, n INT);
 CREATE TABLE join_large (i INT, s STRING, INDEX (i) WHERE s IN ('foo', 'bar', 'baz'));
 ALTER TABLE join_small INJECT STATISTICS '[

pkg/sql/logictest/testdata/logic_test/pg_catalog

@@ -1,8 +1,5 @@
 # LogicTest: local
 
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 # Verify pg_catalog database handles mutation statements correctly.
 
 query error database "pg_catalog" does not exist

pkg/sql/logictest/testdata/logic_test/privileges_comments

@@ -1,7 +1,3 @@
-# Disable automatic stats to avoid flakiness.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 subtest regression45707
 
 user root

pkg/sql/logictest/testdata/logic_test/privileges_table

@@ -1,7 +1,3 @@
-# Disable automatic stats to avoid flakiness.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 # Test default table-level permissions.
 # Default user is root.
 statement ok

pkg/sql/logictest/testdata/logic_test/schema_change_in_txn

@@ -1,8 +1,3 @@
-# Disable automatic stats to avoid flakiness (sometimes causes retry errors).
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
-
 # Skip the rest of the test if a retry occurs. They can happen and are fine
 # but there's no way to encapsulate that in logictests.
 skip_on_retry

pkg/sql/logictest/testdata/logic_test/select

@@ -731,9 +731,6 @@ SELECT * FROM t44132 WHERE c0
 true NULL
 
 # Tests for `disallow_full_table_scans`
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 statement ok
 CREATE TABLE t_disallow_scans(a INT, b INT, INDEX b_idx(b), INDEX b_partial(b) WHERE a > 0);
 
@@ -842,7 +839,6 @@ SELECT * FROM t_disallow_scans@b_idx
 statement ok
 SET disallow_full_table_scans = false;
 RESET large_full_scan_rows;
-RESET CLUSTER SETTING sql.stats.automatic_collection.enabled;
 
 # Regression test for #58104.
 statement ok

pkg/sql/logictest/testdata/logic_test/vectorize

@@ -3,10 +3,6 @@
 # Note that there is no much benefit from running this file with other logic
 # test configurations because we override vectorize setting.
 
-# Disable automatic stats.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 # NOTE: all queries in this file should run with vectorize=experimental_always
 # unless they are known to be unsupported (like generate_series, etc). If you do
 # need to execute an unsupported query, follow the pattern of

pkg/sql/opt/exec/execbuilder/testdata/distsql_single_flow

@@ -1,8 +1,5 @@
 # LogicTest: 5node
 
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 statement ok
 CREATE TABLE t (a INT PRIMARY KEY, b INT, c INT)
 

pkg/sql/opt/exec/execbuilder/testdata/stats

@@ -3,10 +3,6 @@
 # Tests that verify we retrieve the stats correctly. Note that we can't create
 # statistics if distsql mode is OFF.
 
-# Disable automatic stats to prevent flakes if auto stats run.
-statement ok
-SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
-
 statement ok
 CREATE TABLE uv (u INT, v INT, INDEX (u) STORING (v), INDEX (v) STORING (u));
 INSERT INTO uv VALUES (1, 1), (1, 2), (1, 3), (1, 4), (2, 4), (2, 5), (2, 6), (2, 7)