diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 04533af09283..c7ee9952d237 100644 --- a/pkg/sql/logictest/logic.go +++ b/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: "local-v1.1@v1.0-noupgrade", numNodes: 1, overrideDistSQLMode: "off", - overrideAutoStats: "false", bootstrapVersion: roachpb.Version{Major: 1}, binaryVersion: roachpb.Version{Major: 1, Minor: 1}, disableUpgrade: true, @@ -702,7 +697,6 @@ var logicTestConfigs = []testClusterConfig{ name: "local-spec-planning", numNodes: 1, overrideDistSQLMode: "off", - overrideAutoStats: "false", overrideExperimentalDistSQLPlanning: "on", }, { @@ -710,14 +704,12 @@ var logicTestConfigs = []testClusterConfig{ numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", - overrideAutoStats: "false", }, { name: "fakedist-vec-off", numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", - overrideAutoStats: "false", overrideVectorize: "off", }, { @@ -725,7 +717,6 @@ var logicTestConfigs = []testClusterConfig{ 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,20 +733,17 @@ 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, }, @@ -764,7 +751,6 @@ var logicTestConfigs = []testClusterConfig{ name: "5node-disk", numNodes: 5, overrideDistSQLMode: "on", - overrideAutoStats: "false", sqlExecUseDisk: true, skipShort: true, }, @@ -772,7 +758,6 @@ var logicTestConfigs = []testClusterConfig{ name: "5node-spec-planning", numNodes: 5, overrideDistSQLMode: "on", - overrideAutoStats: "false", overrideExperimentalDistSQLPlanning: "on", }, { @@ -781,13 +766,10 @@ 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// - 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 @@ -795,9 +777,8 @@ var logicTestConfigs = []testClusterConfig{ // 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 != "" { diff --git a/pkg/sql/logictest/testdata/logic_test/delete b/pkg/sql/logictest/testdata/logic_test/delete index dc1d5a2db7c5..719d5dc5721b 100644 --- a/pkg/sql/logictest/testdata/logic_test/delete +++ b/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) diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_event_log b/pkg/sql/logictest/testdata/logic_test/distsql_event_log index e979e5b9a1ef..7423d1d99245 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_event_log +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_stats b/pkg/sql/logictest/testdata/logic_test/distsql_stats index 903dac6f5a2f..ca78f57c67c9 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_stats +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/index_join b/pkg/sql/logictest/testdata/logic_test/index_join index 747c259b4f26..db26c6c264e2 100644 --- a/pkg/sql/logictest/testdata/logic_test/index_join +++ b/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, diff --git a/pkg/sql/logictest/testdata/logic_test/inv_stats b/pkg/sql/logictest/testdata/logic_test/inv_stats index f043059c1d80..ca8ab06016a2 100644 --- a/pkg/sql/logictest/testdata/logic_test/inv_stats +++ b/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); diff --git a/pkg/sql/logictest/testdata/logic_test/partial_index b/pkg/sql/logictest/testdata/logic_test/partial_index index 2208770966a1..ce153bc940d6 100644 --- a/pkg/sql/logictest/testdata/logic_test/partial_index +++ b/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 '[ diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index 545360c6d31a..ed135036625b 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/privileges_comments b/pkg/sql/logictest/testdata/logic_test/privileges_comments index 54fd1dbb57ae..17752cc35589 100644 --- a/pkg/sql/logictest/testdata/logic_test/privileges_comments +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/privileges_table b/pkg/sql/logictest/testdata/logic_test/privileges_table index aee1dd2e8655..a1ed704d55b2 100644 --- a/pkg/sql/logictest/testdata/logic_test/privileges_table +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn index fb3e9f78f4cb..fc2591c63e07 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/select b/pkg/sql/logictest/testdata/logic_test/select index 9da81fe86118..573809742d30 100644 --- a/pkg/sql/logictest/testdata/logic_test/select +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/vectorize b/pkg/sql/logictest/testdata/logic_test/vectorize index 696b05ca8f31..eb3d31862540 100644 --- a/pkg/sql/logictest/testdata/logic_test/vectorize +++ b/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 diff --git a/pkg/sql/opt/exec/execbuilder/testdata/distsql_single_flow b/pkg/sql/opt/exec/execbuilder/testdata/distsql_single_flow index 38b3b39c6ff2..4a39dd6700a4 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/distsql_single_flow +++ b/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) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/stats b/pkg/sql/opt/exec/execbuilder/testdata/stats index 587f0534687f..52383920b889 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/stats +++ b/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) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/tpch_vec b/pkg/sql/opt/exec/execbuilder/testdata/tpch_vec index 8b41bcf4c901..05f3802c4014 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/tpch_vec +++ b/pkg/sql/opt/exec/execbuilder/testdata/tpch_vec @@ -1,8 +1,6 @@ # LogicTest: local # Note that statistics are populated for TPCH Scale Factor 1. -statement ok -SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false statement ok CREATE TABLE public.region