From 1bb278361af369c4b8c084fe3e16d7becf6ec1ae Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 24 Jan 2024 11:44:52 -0800 Subject: [PATCH 1/4] sql/tests: remove stale skipped error We now have key-encoding of JSONs. Epic: None Release note: None --- pkg/sql/tests/rsg_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sql/tests/rsg_test.go b/pkg/sql/tests/rsg_test.go index 29defd3fe447..dfe9b99957c1 100644 --- a/pkg/sql/tests/rsg_test.go +++ b/pkg/sql/tests/rsg_test.go @@ -618,7 +618,6 @@ var ignoredErrorPatterns = []string{ "cannot call json_object_keys on an array", "cannot set path in scalar", "cannot delete path in scalar", - "unable to encode table key: \\*tree\\.DJSON", "path element at position .* is null", "path element is not an integer", "cannot delete from object using integer index", From 969ee01175ed4a9420ab610e4236d946ec271f98 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Thu, 25 Jan 2024 12:44:37 -0800 Subject: [PATCH 2/4] ttl: use better defaults for some session variables This commit fixes some problems that we've seen around internally-executed queries issued by the TTL jobs, present before 23.2. In particular, on 23.1 and prior releases we used Go defaults for the session data parameters for internally-executed queries, which can differ from the defaults set by CRDB. As a result, query plans could be suboptimal as we've seen in a couple of recent escalations. This bug has been fixed on 23.2 proper (we now use CRDB defaults for all session variables except for one), and on 23.1 and prior this commit applies a targeted fix only to the TTL queries. In particular, the following overrides are now set: - `reorder_joins_limit` to the default value of 8 - `optimizer_use_histograms` and `optimizer_use_multi_col_stats` are both set to `true`. On 23.2 and later only `optimizer_use_histograms` needs to be updated since it's the only exception mentioned above. However, I chose to keep all 3 in the same commit so that it can be backported more easily to 23.1, and the following commit will revert the other 2 on 23.2 and later. Release note (bug fix): Internal queries issued by the TTL jobs should now use optimal plans. The bug has been present since at least 22.2 version. --- pkg/sql/internal.go | 9 +++++++++ pkg/sql/sessiondata/internal.go | 9 +++++++++ pkg/sql/ttl/ttljob/BUILD.bazel | 1 + pkg/sql/ttl/ttljob/ttljob_metrics.go | 7 +------ pkg/sql/ttl/ttljob/ttljob_processor.go | 2 ++ pkg/sql/ttl/ttljob/ttljob_query_builder.go | 23 ++++++++++++++-------- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/pkg/sql/internal.go b/pkg/sql/internal.go index 6b8c318dda2c..5f5195d4ec14 100644 --- a/pkg/sql/internal.go +++ b/pkg/sql/internal.go @@ -863,6 +863,15 @@ func applyOverrides(o sessiondata.InternalExecutorOverride, sd *sessiondata.Sess } // We always override the injection knob based on the override struct. sd.InjectRetryErrorsEnabled = o.InjectRetryErrorsEnabled + if o.ReorderJoinsLimit != 0 { + sd.ReorderJoinsLimit = o.ReorderJoinsLimit + } + if o.OptimizerUseHistograms { + sd.OptimizerUseHistograms = true + } + if o.OptimizerUseMultiColStats { + sd.OptimizerUseMultiColStats = true + } } func (ie *InternalExecutor) maybeRootSessionDataOverride( diff --git a/pkg/sql/sessiondata/internal.go b/pkg/sql/sessiondata/internal.go index 0c2590fe2894..1a3494c44310 100644 --- a/pkg/sql/sessiondata/internal.go +++ b/pkg/sql/sessiondata/internal.go @@ -47,6 +47,15 @@ type InternalExecutorOverride struct { // does **not** propagate further to "nested" executors that are spawned up // by the "top" executor. InjectRetryErrorsEnabled bool + // ReorderJoinsLimit indicates the number of joins at which the optimizer + // should stop attempting to reorder. + ReorderJoinsLimit int64 + // OptimizerUseHistograms indicates whether we should use histograms for + // cardinality estimation in the optimizer. + OptimizerUseHistograms bool + // OptimizerUseMultiColStats indicates whether we should use multi-column + // statistics for cardinality estimation in the optimizer. + OptimizerUseMultiColStats bool } // NoSessionDataOverride is the empty InternalExecutorOverride which does not diff --git a/pkg/sql/ttl/ttljob/BUILD.bazel b/pkg/sql/ttl/ttljob/BUILD.bazel index adf135c505fd..34a06880b92b 100644 --- a/pkg/sql/ttl/ttljob/BUILD.bazel +++ b/pkg/sql/ttl/ttljob/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "//pkg/sql/execinfrapb", "//pkg/sql/isql", "//pkg/sql/lexbase", + "//pkg/sql/opt", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/physicalplan", diff --git a/pkg/sql/ttl/ttljob/ttljob_metrics.go b/pkg/sql/ttl/ttljob/ttljob_metrics.go index 1746a0ed4661..095c36ab869d 100644 --- a/pkg/sql/ttl/ttljob/ttljob_metrics.go +++ b/pkg/sql/ttl/ttljob/ttljob_metrics.go @@ -18,11 +18,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" - "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/metric/aggmetric" @@ -225,10 +223,7 @@ func (m *rowLevelTTLMetrics) fetchStatistics( ctx, c.opName, nil, - sessiondata.InternalExecutorOverride{ - User: username.RootUserName(), - QualityOfService: &qosLevel, - }, + getInternalExecutorOverride(qosLevel), fmt.Sprintf(c.query, details.TableID, aost.String()), c.args..., ) diff --git a/pkg/sql/ttl/ttljob/ttljob_processor.go b/pkg/sql/ttl/ttljob/ttljob_processor.go index 914120a8ba3b..ae41c83ec06f 100644 --- a/pkg/sql/ttl/ttljob/ttljob_processor.go +++ b/pkg/sql/ttl/ttljob/ttljob_processor.go @@ -301,6 +301,8 @@ func (t *ttlProcessor) runTTLOnQueryBounds( ctx, "pre-select-delete-statement", nil, /* txn */ + // This is a test-only knob, so we're ok not specifying custom + // InternalExecutorOverride. sessiondata.RootUserSessionDataOverride, preSelectStatement, ); err != nil { diff --git a/pkg/sql/ttl/ttljob/ttljob_query_builder.go b/pkg/sql/ttl/ttljob/ttljob_query_builder.go index dc28b8ca373e..9ee1f838f93e 100644 --- a/pkg/sql/ttl/ttljob/ttljob_query_builder.go +++ b/pkg/sql/ttl/ttljob/ttljob_query_builder.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/isql" + "github.com/cockroachdb/cockroach/pkg/sql/opt" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" @@ -120,6 +121,18 @@ func (b *SelectQueryBuilder) buildQuery() string { var qosLevel = sessiondatapb.TTLLow +func getInternalExecutorOverride( + qosLevel sessiondatapb.QoSLevel, +) sessiondata.InternalExecutorOverride { + return sessiondata.InternalExecutorOverride{ + User: username.RootUserName(), + QualityOfService: &qosLevel, + ReorderJoinsLimit: opt.DefaultJoinOrderLimit, + OptimizerUseHistograms: true, + OptimizerUseMultiColStats: true, + } +} + func (b *SelectQueryBuilder) Run( ctx context.Context, ie isql.Executor, ) (_ []tree.Datums, hasNext bool, _ error) { @@ -148,10 +161,7 @@ func (b *SelectQueryBuilder) Run( ctx, b.selectOpName, nil, /* txn */ - sessiondata.InternalExecutorOverride{ - User: username.RootUserName(), - QualityOfService: &qosLevel, - }, + getInternalExecutorOverride(qosLevel), query, b.cachedArgs..., ) @@ -254,10 +264,7 @@ func (b *DeleteQueryBuilder) Run( ctx, b.deleteOpName, txn.KV(), - sessiondata.InternalExecutorOverride{ - User: username.RootUserName(), - QualityOfService: &qosLevel, - }, + getInternalExecutorOverride(qosLevel), query, deleteArgs..., ) From 53437c2b1c068528c9f1321ac68ed10e32864b89 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Thu, 25 Jan 2024 13:35:10 -0800 Subject: [PATCH 3/4] ttl: remove a couple of redundant overrides This commit removes the overrides of `reorder_joins_limit` and `optimizer_use_multi_col_stats` in the TTL jobs since these values are set correctly already. Release note: None --- pkg/sql/internal.go | 6 ------ pkg/sql/sessiondata/internal.go | 7 +------ pkg/sql/ttl/ttljob/BUILD.bazel | 1 - pkg/sql/ttl/ttljob/ttljob_query_builder.go | 9 +++------ 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/pkg/sql/internal.go b/pkg/sql/internal.go index 5f5195d4ec14..4d90d39d6453 100644 --- a/pkg/sql/internal.go +++ b/pkg/sql/internal.go @@ -863,15 +863,9 @@ func applyOverrides(o sessiondata.InternalExecutorOverride, sd *sessiondata.Sess } // We always override the injection knob based on the override struct. sd.InjectRetryErrorsEnabled = o.InjectRetryErrorsEnabled - if o.ReorderJoinsLimit != 0 { - sd.ReorderJoinsLimit = o.ReorderJoinsLimit - } if o.OptimizerUseHistograms { sd.OptimizerUseHistograms = true } - if o.OptimizerUseMultiColStats { - sd.OptimizerUseMultiColStats = true - } } func (ie *InternalExecutor) maybeRootSessionDataOverride( diff --git a/pkg/sql/sessiondata/internal.go b/pkg/sql/sessiondata/internal.go index 1a3494c44310..114c33dec950 100644 --- a/pkg/sql/sessiondata/internal.go +++ b/pkg/sql/sessiondata/internal.go @@ -47,15 +47,10 @@ type InternalExecutorOverride struct { // does **not** propagate further to "nested" executors that are spawned up // by the "top" executor. InjectRetryErrorsEnabled bool - // ReorderJoinsLimit indicates the number of joins at which the optimizer - // should stop attempting to reorder. - ReorderJoinsLimit int64 // OptimizerUseHistograms indicates whether we should use histograms for // cardinality estimation in the optimizer. + // TODO(#102954): this should be removed when #102954 is fixed. OptimizerUseHistograms bool - // OptimizerUseMultiColStats indicates whether we should use multi-column - // statistics for cardinality estimation in the optimizer. - OptimizerUseMultiColStats bool } // NoSessionDataOverride is the empty InternalExecutorOverride which does not diff --git a/pkg/sql/ttl/ttljob/BUILD.bazel b/pkg/sql/ttl/ttljob/BUILD.bazel index 34a06880b92b..adf135c505fd 100644 --- a/pkg/sql/ttl/ttljob/BUILD.bazel +++ b/pkg/sql/ttl/ttljob/BUILD.bazel @@ -30,7 +30,6 @@ go_library( "//pkg/sql/execinfrapb", "//pkg/sql/isql", "//pkg/sql/lexbase", - "//pkg/sql/opt", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/physicalplan", diff --git a/pkg/sql/ttl/ttljob/ttljob_query_builder.go b/pkg/sql/ttl/ttljob/ttljob_query_builder.go index 9ee1f838f93e..b95641010e41 100644 --- a/pkg/sql/ttl/ttljob/ttljob_query_builder.go +++ b/pkg/sql/ttl/ttljob/ttljob_query_builder.go @@ -19,7 +19,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/isql" - "github.com/cockroachdb/cockroach/pkg/sql/opt" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" @@ -125,11 +124,9 @@ func getInternalExecutorOverride( qosLevel sessiondatapb.QoSLevel, ) sessiondata.InternalExecutorOverride { return sessiondata.InternalExecutorOverride{ - User: username.RootUserName(), - QualityOfService: &qosLevel, - ReorderJoinsLimit: opt.DefaultJoinOrderLimit, - OptimizerUseHistograms: true, - OptimizerUseMultiColStats: true, + User: username.RootUserName(), + QualityOfService: &qosLevel, + OptimizerUseHistograms: true, } } From dae54bd17c23bdc9ea05de9f17f27911f934bb84 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 30 Jan 2024 11:03:09 -0500 Subject: [PATCH 4/4] sql: enable read committed cluster setting by default Release note (sql change): The sql.txn.read_committed_isolation.enabled cluster setting is now true by default. This means that any syntax and settings that configure the READ COMMITTED isolation level will now cause the transaction to use that isolation level, rather than automatically upgrading the transaction to SERIALIZABLE. --- docs/generated/settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- .../logic_test/regional_by_row_read_committed | 3 --- pkg/cmd/roachtest/tests/orm_helpers.go | 1 - pkg/cmd/roachtest/tests/ycsb.go | 1 - pkg/sql/conn_executor.go | 2 +- pkg/sql/conn_executor_test.go | 5 +---- .../logictest/testdata/logic_test/cluster_locks | 3 --- .../testdata/logic_test/fk_read_committed | 3 --- .../logic_test/hash_sharded_index_read_committed | 3 --- .../logictest/testdata/logic_test/read_committed | 3 --- .../logic_test/select_for_update_read_committed | 3 --- pkg/sql/logictest/testdata/logic_test/txn | 7 ++----- .../testdata/logic_test/unique_read_committed | 3 --- .../opt/exec/execbuilder/testdata/explain_analyze | 3 --- .../exec/execbuilder/testdata/fk_read_committed | 3 --- .../testdata/select_for_update_read_committed | 3 --- .../execbuilder/testdata/unique_read_committed | 3 --- .../execbuilder/testdata/update_read_committed | 3 --- .../execbuilder/testdata/upsert_read_committed | 3 --- pkg/sql/pgwire/conn_test.go | 3 --- pkg/sql/pgwire/testdata/pgtest/read_committed | 4 ---- pkg/sql/tests/read_committed_test.go | 14 +++----------- 23 files changed, 9 insertions(+), 71 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 002a74be8f60..787324f828a0 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -317,7 +317,7 @@ sql.ttl.default_delete_rate_limit integer 0 default delete rate limit (rows per sql.ttl.default_select_batch_size integer 500 default amount of rows to select in a single query during a TTL job application sql.ttl.default_select_rate_limit integer 0 default select rate limit (rows per second) per node for each TTL job. Use 0 to signify no rate limit. application sql.ttl.job.enabled boolean true whether the TTL job is enabled application -sql.txn.read_committed_isolation.enabled boolean false set to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commands application +sql.txn.read_committed_isolation.enabled boolean true set to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commands application sql.txn_fingerprint_id_cache.capacity integer 100 the maximum number of txn fingerprint IDs stored application storage.max_sync_duration duration 20s maximum duration for disk operations; any operations that take longer than this setting trigger a warning log entry or process crash system-visible storage.max_sync_duration.fatal.enabled boolean true if true, fatal the process when a disk operation exceeds storage.max_sync_duration application diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index d2fe1b6f21e9..516409c5aebf 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -267,7 +267,7 @@
sql.ttl.default_select_batch_size
integer500default amount of rows to select in a single query during a TTL jobServerless/Dedicated/Self-Hosted
sql.ttl.default_select_rate_limit
integer0default select rate limit (rows per second) per node for each TTL job. Use 0 to signify no rate limit.Serverless/Dedicated/Self-Hosted
sql.ttl.job.enabled
booleantruewhether the TTL job is enabledServerless/Dedicated/Self-Hosted -
sql.txn.read_committed_isolation.enabled
booleanfalseset to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commandsServerless/Dedicated/Self-Hosted +
sql.txn.read_committed_isolation.enabled
booleantrueset to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commandsServerless/Dedicated/Self-Hosted
sql.txn_fingerprint_id_cache.capacity
integer100the maximum number of txn fingerprint IDs storedServerless/Dedicated/Self-Hosted
storage.experimental.eventually_file_only_snapshots.enabled
booleantrueset to false to disable eventually-file-only-snapshots (kv.snapshot_receiver.excise.enabled must also be false)Dedicated/Self-Hosted
storage.ingest_split.enabled
booleantrueset to false to disable ingest-time splitting that lowers write-amplificationDedicated/Self-Hosted diff --git a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_read_committed b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_read_committed index bc912a4cad36..c6e6bca5a19f 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_read_committed +++ b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_read_committed @@ -1,9 +1,6 @@ # tenant-cluster-setting-override-opt: sql.zone_configs.allow_for_secondary_tenant.enabled=true sql.multi_region.allow_abstractions_for_secondary_tenants.enabled=true # LogicTest: multiregion-9node-3region-3azs multiregion-9node-3region-3azs-vec-off multiregion-9node-3region-3azs-tenant multiregion-9node-3region-3azs-no-los -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED diff --git a/pkg/cmd/roachtest/tests/orm_helpers.go b/pkg/cmd/roachtest/tests/orm_helpers.go index c6e5fdf9d47e..097821203288 100644 --- a/pkg/cmd/roachtest/tests/orm_helpers.go +++ b/pkg/cmd/roachtest/tests/orm_helpers.go @@ -71,7 +71,6 @@ func alterZoneConfigAndClusterSettings( // Enable experimental/preview/compatibility features. `SET CLUSTER SETTING sql.defaults.experimental_temporary_tables.enabled = 'true';`, - `SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = 'true';`, `ALTER ROLE ALL SET multiple_active_portals_enabled = 'true';`, `ALTER ROLE ALL SET serial_normalization = 'sql_sequence_cached'`, `ALTER ROLE ALL SET statement_timeout = '60s'`, diff --git a/pkg/cmd/roachtest/tests/ycsb.go b/pkg/cmd/roachtest/tests/ycsb.go index 47180d24e12e..4dd67fb95b19 100644 --- a/pkg/cmd/roachtest/tests/ycsb.go +++ b/pkg/cmd/roachtest/tests/ycsb.go @@ -168,7 +168,6 @@ func registerYCSB(r registry.Registry) { func enableIsolationLevels(ctx context.Context, t test.Test, db *gosql.DB) error { for _, cmd := range []string{ - `SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = 'true';`, `SET CLUSTER SETTING sql.txn.snapshot_isolation.enabled = 'true';`, } { if _, err := db.ExecContext(ctx, cmd); err != nil { diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index 3223cbf2a2bd..5fc38842265a 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -3426,7 +3426,7 @@ var allowReadCommittedIsolation = settings.RegisterBoolSetting( "sql.txn.read_committed_isolation.enabled", "set to true to allow transactions to use the READ COMMITTED isolation "+ "level if specified by BEGIN/SET commands", - false, + true, settings.WithPublic, ) diff --git a/pkg/sql/conn_executor_test.go b/pkg/sql/conn_executor_test.go index 05dce250a1e7..65167fe50dd3 100644 --- a/pkg/sql/conn_executor_test.go +++ b/pkg/sql/conn_executor_test.go @@ -427,7 +427,6 @@ func TestHalloweenProblemAvoidance(t *testing.T) { defer s.Stopper().Stop(context.Background()) for _, s := range []string{ - `SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true;`, `SET CLUSTER SETTING sql.txn.snapshot_isolation.enabled = true;`, `CREATE DATABASE t;`, `CREATE TABLE t.test (x FLOAT);`, @@ -1470,9 +1469,7 @@ func TestInjectRetryErrors(t *testing.T) { defer s.Stopper().Stop(ctx) defer db.Close() - _, err := db.Exec(`SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true`) - require.NoError(t, err) - _, err = db.Exec("SET inject_retry_errors_enabled = 'true'") + _, err := db.Exec("SET inject_retry_errors_enabled = 'true'") require.NoError(t, err) t.Run("with_savepoints", func(t *testing.T) { diff --git a/pkg/sql/logictest/testdata/logic_test/cluster_locks b/pkg/sql/logictest/testdata/logic_test/cluster_locks index 13ba578f5288..7e37a37ef48f 100644 --- a/pkg/sql/logictest/testdata/logic_test/cluster_locks +++ b/pkg/sql/logictest/testdata/logic_test/cluster_locks @@ -350,9 +350,6 @@ SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name IN ('t','t2') # Test with different isolation levels. -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok SET CLUSTER SETTING sql.txn.snapshot_isolation.enabled = true diff --git a/pkg/sql/logictest/testdata/logic_test/fk_read_committed b/pkg/sql/logictest/testdata/logic_test/fk_read_committed index 3f02035103e3..94e85605b0f7 100644 --- a/pkg/sql/logictest/testdata/logic_test/fk_read_committed +++ b/pkg/sql/logictest/testdata/logic_test/fk_read_committed @@ -1,6 +1,3 @@ -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok CREATE TABLE jars (j INT PRIMARY KEY) diff --git a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index_read_committed b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index_read_committed index 4aa931839c26..78f17c6982f4 100644 --- a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index_read_committed +++ b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index_read_committed @@ -1,6 +1,3 @@ -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED diff --git a/pkg/sql/logictest/testdata/logic_test/read_committed b/pkg/sql/logictest/testdata/logic_test/read_committed index cdaf8653cffa..01d342b0f19a 100644 --- a/pkg/sql/logictest/testdata/logic_test/read_committed +++ b/pkg/sql/logictest/testdata/logic_test/read_committed @@ -1,8 +1,5 @@ # LogicTest: !local-mixed-23.1 -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - subtest select_for_update statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed b/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed index fb581048d0af..0f4adedfb170 100644 --- a/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed +++ b/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed @@ -1,8 +1,5 @@ # LogicTest: !local-mixed-23.1 -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok CREATE TABLE abc (a INT PRIMARY KEY, b INT, c INT, INDEX (b), FAMILY (a, b, c)) diff --git a/pkg/sql/logictest/testdata/logic_test/txn b/pkg/sql/logictest/testdata/logic_test/txn index b6e0cd7c462f..3e948bed7f16 100644 --- a/pkg/sql/logictest/testdata/logic_test/txn +++ b/pkg/sql/logictest/testdata/logic_test/txn @@ -207,9 +207,6 @@ BEGIN TRANSACTION; SET TRANSACTION ISOLATION LEVEL READ COMMITTED; COMMIT # It is an error to change the isolation level of a running transaction. -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok BEGIN TRANSACTION @@ -237,9 +234,9 @@ statement ok ROLLBACK statement ok -RESET CLUSTER SETTING sql.txn.read_committed_isolation.enabled +SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = false -# Transactions default to serializable. +# Transactions default to serializable when the read_committed cluster setting is off. statement ok BEGIN TRANSACTION diff --git a/pkg/sql/logictest/testdata/logic_test/unique_read_committed b/pkg/sql/logictest/testdata/logic_test/unique_read_committed index 2e19aafea96a..0ad1ee2ef137 100644 --- a/pkg/sql/logictest/testdata/logic_test/unique_read_committed +++ b/pkg/sql/logictest/testdata/logic_test/unique_read_committed @@ -1,8 +1,5 @@ # LogicTest: !local-mixed-23.1 -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok SET experimental_enable_unique_without_index_constraints = true diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze b/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze index 60740d987605..056a05590adf 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok CREATE TABLE kv (k INT PRIMARY KEY, v INT, FAMILY (k, v)) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/fk_read_committed b/pkg/sql/opt/exec/execbuilder/testdata/fk_read_committed index 154d726b73e4..bfb354780592 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/fk_read_committed +++ b/pkg/sql/opt/exec/execbuilder/testdata/fk_read_committed @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok CREATE TABLE jars (j INT PRIMARY KEY) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/select_for_update_read_committed b/pkg/sql/opt/exec/execbuilder/testdata/select_for_update_read_committed index 79ba9ce01d69..dbf7c31f5a4a 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/select_for_update_read_committed +++ b/pkg/sql/opt/exec/execbuilder/testdata/select_for_update_read_committed @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok CREATE TABLE supermarket ( person STRING PRIMARY KEY, diff --git a/pkg/sql/opt/exec/execbuilder/testdata/unique_read_committed b/pkg/sql/opt/exec/execbuilder/testdata/unique_read_committed index c1e5ad4c171f..f1f700594d8e 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/unique_read_committed +++ b/pkg/sql/opt/exec/execbuilder/testdata/unique_read_committed @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - statement ok SET experimental_enable_unique_without_index_constraints = true diff --git a/pkg/sql/opt/exec/execbuilder/testdata/update_read_committed b/pkg/sql/opt/exec/execbuilder/testdata/update_read_committed index 4492b54a1c76..7f19c8202d9f 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/update_read_committed +++ b/pkg/sql/opt/exec/execbuilder/testdata/update_read_committed @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - # Test multi-column-family checks under read committed. If a check constraint # contains multiple column families but only some of them are updated, the # query should fail under read committed. diff --git a/pkg/sql/opt/exec/execbuilder/testdata/upsert_read_committed b/pkg/sql/opt/exec/execbuilder/testdata/upsert_read_committed index 21f04bfbccab..202c770af9bb 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/upsert_read_committed +++ b/pkg/sql/opt/exec/execbuilder/testdata/upsert_read_committed @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true - # Test multi-column-family checks under read committed. If a check constraint # contains multiple column families but only some of them are updated, the # query should fail under read committed. diff --git a/pkg/sql/pgwire/conn_test.go b/pkg/sql/pgwire/conn_test.go index f255803a3dbb..d0eb1a813644 100644 --- a/pkg/sql/pgwire/conn_test.go +++ b/pkg/sql/pgwire/conn_test.go @@ -1767,9 +1767,6 @@ func TestSetSessionArguments(t *testing.T) { defer srv.Stopper().Stop(ctx) s := srv.ApplicationLayer() - _, err := s.SQLConn(t, serverutils.DBName("defaultdb")).Exec(`SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true`) - require.NoError(t, err) - pgURL, cleanupFunc := s.PGUrl( t, serverutils.CertsDirPrefix("testConnClose"), serverutils.User(username.RootUser), ) diff --git a/pkg/sql/pgwire/testdata/pgtest/read_committed b/pkg/sql/pgwire/testdata/pgtest/read_committed index c4660e5828ac..d53d19ab65b8 100644 --- a/pkg/sql/pgwire/testdata/pgtest/read_committed +++ b/pkg/sql/pgwire/testdata/pgtest/read_committed @@ -2,16 +2,12 @@ only crdb ---- send -Query {"String": "SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = 'true'"} Query {"String": "SET default_transaction_isolation = 'read committed'"} ---- until ReadyForQuery -ReadyForQuery ---- -{"Type":"CommandComplete","CommandTag":"SET CLUSTER SETTING"} -{"Type":"ReadyForQuery","TxStatus":"I"} {"Type":"CommandComplete","CommandTag":"SET"} {"Type":"ReadyForQuery","TxStatus":"I"} diff --git a/pkg/sql/tests/read_committed_test.go b/pkg/sql/tests/read_committed_test.go index 75908624e1c6..0d53effef46a 100644 --- a/pkg/sql/tests/read_committed_test.go +++ b/pkg/sql/tests/read_committed_test.go @@ -89,12 +89,9 @@ func TestReadCommittedStmtRetry(t *testing.T) { defer s.Stopper().Stop(ctx) codec = s.ApplicationLayer().Codec() - _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true`) - require.NoError(t, err) - // Create a table with three rows. Note that k is not the primary key, // so locking won't be pushed into the initial scan of the UPDATEs below. - _, err = sqlDB.Exec(`CREATE TABLE kv (k TEXT, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) + _, err := sqlDB.Exec(`CREATE TABLE kv (k TEXT, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) require.NoError(t, err) _, err = sqlDB.Exec(`INSERT INTO kv VALUES ('a', 1);`) require.NoError(t, err) @@ -181,9 +178,7 @@ func TestReadCommittedReadTimestampNotSteppedOnCommit(t *testing.T) { s, sqlDB, _ := serverutils.StartServer(t, params) defer s.Stopper().Stop(ctx) - _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true`) - require.NoError(t, err) - _, err = sqlDB.Exec(`CREATE TABLE kv (k TEXT, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) + _, err := sqlDB.Exec(`CREATE TABLE kv (k TEXT, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) require.NoError(t, err) // Create a read committed transaction that writes to three rows in three @@ -226,10 +221,7 @@ func TestReadCommittedVolatileUDF(t *testing.T) { s, sqlDB, _ := serverutils.StartServer(t, params) defer s.Stopper().Stop(ctx) - _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true`) - require.NoError(t, err) - - _, err = sqlDB.Exec(`CREATE TABLE kv (k TEXT PRIMARY KEY, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) + _, err := sqlDB.Exec(`CREATE TABLE kv (k TEXT PRIMARY KEY, v INT) WITH (sql_stats_automatic_collection_enabled = false);`) require.NoError(t, err) _, err = sqlDB.Exec(`INSERT INTO kv VALUES ('a', 10);`) require.NoError(t, err)