Skip to content

Commit 0b1c685

Browse files
craig[bot]rafiss
craig[bot]
and
rafiss
committed
108137: sql: promote implicit RC txn with DDL to SERIALIZABLE r=rafiss a=rafiss We do not currently support schema changes in READ COMMITTED transactions. This change makes it so implicit transactions that are running under READ COMMITTED are automatically promoted to SERIALIZABLE if it has a schema change. This reduces the amount of backwards incompatibility with rolling out READ COMMITTED. Informs: cockroachdb#107980 informs: cockroachdb#107683 Epic: CRDB-26546 Release note: None Co-authored-by: Rafi Shamim <[email protected]>
2 parents 886515f + 7df3b47 commit 0b1c685

File tree

3 files changed

+199
-18
lines changed

3 files changed

+199
-18
lines changed

‎pkg/sql/conn_executor_exec.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"github.com/cockroachdb/cockroach/pkg/jobs"
2323
"github.com/cockroachdb/cockroach/pkg/kv"
24+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
2425
"github.com/cockroachdb/cockroach/pkg/multitenant"
2526
"github.com/cockroachdb/cockroach/pkg/multitenant/multitenantcpu"
2627
"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -40,6 +41,7 @@ import (
4041
"github.com/cockroachdb/cockroach/pkg/sql/parser/statements"
4142
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
4243
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
44+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
4345
"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
4446
"github.com/cockroachdb/cockroach/pkg/sql/sem/asof"
4547
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
@@ -1966,6 +1968,21 @@ func (ex *connExecutor) handleTxnRowsWrittenReadLimits(ctx context.Context) erro
19661968
// makeExecPlan creates an execution plan and populates planner.curPlan using
19671969
// the cost-based optimizer.
19681970
func (ex *connExecutor) makeExecPlan(ctx context.Context, planner *planner) error {
1971+
if tree.CanModifySchema(planner.stmt.AST) {
1972+
if planner.Txn().IsoLevel().ToleratesWriteSkew() {
1973+
if planner.extendedEvalCtx.TxnIsSingleStmt && planner.extendedEvalCtx.TxnImplicit {
1974+
if err := ex.state.setIsolationLevel(isolation.Serializable); err != nil {
1975+
return err
1976+
}
1977+
planner.BufferClientNotice(ctx, pgnotice.Newf("setting implicit transaction isolation level to SERIALIZABLE due to schema change"))
1978+
} else {
1979+
return pgerror.Newf(
1980+
pgcode.FeatureNotSupported, "explicit transaction involving a schema change needs to be SERIALIZABLE",
1981+
)
1982+
}
1983+
}
1984+
}
1985+
19691986
if err := planner.makeOptimizerPlan(ctx); err != nil {
19701987
log.VEventf(ctx, 1, "optimizer plan failed: %v", err)
19711988
return err

‎pkg/sql/logictest/testdata/logic_test/read_committed‎

Lines changed: 182 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -147,89 +147,258 @@ SELECT aisle
147147

148148
subtest end
149149

150-
subtest schema_changes
150+
subtest schema_changes_implicit
151151

152-
# Schema changes are prohibited under weaker isolation levels.
152+
# Schema changes in implicit READ COMMITTED transactions cause the transaction
153+
# to be promoted to SERIALIZABLE.
154+
155+
query T noticetrace
156+
ALTER TABLE supermarket ADD COLUMN age INT
157+
----
158+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
159+
160+
query T noticetrace
161+
CREATE TABLE foo(a INT)
162+
----
163+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
164+
165+
query T noticetrace
166+
DROP TABLE supermarket
167+
----
168+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
169+
170+
query T noticetrace
171+
DROP USER testuser
172+
----
173+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
174+
175+
query T noticetrace
176+
CREATE USER testuser
177+
----
178+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
179+
180+
query T noticetrace
181+
GRANT admin TO testuser
182+
----
183+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
184+
185+
query T noticetrace
186+
GRANT SELECT ON foo TO testuser
187+
----
188+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
189+
190+
query T noticetrace
191+
GRANT USAGE ON SCHEMA public TO testuser
192+
----
193+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
194+
195+
query T noticetrace
196+
CREATE INDEX foo_idx ON foo(a)
197+
----
198+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
199+
200+
query T noticetrace
201+
CREATE FUNCTION f (x INT) RETURNS INT LANGUAGE SQL AS $$
202+
SELECT x+1
203+
$$
204+
----
205+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
206+
207+
query T noticetrace
208+
ALTER FUNCTION f (x INT) RENAME TO g
209+
----
210+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
211+
212+
query T noticetrace
213+
GRANT EXECUTE ON FUNCTION g (x INT) TO testuser
214+
----
215+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
216+
217+
query T noticetrace
218+
CREATE TYPE typ AS ENUM('a', 'b')
219+
----
220+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
221+
222+
query T noticetrace
223+
ALTER TYPE typ ADD VALUE 'c'
224+
----
225+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
226+
227+
query T noticetrace
228+
GRANT USAGE ON TYPE typ TO testuser
229+
----
230+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
231+
232+
query T noticetrace
233+
CREATE DATABASE foo
234+
----
235+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
236+
237+
query T noticetrace
238+
GRANT CONNECT ON DATABASE foo TO testuser
239+
----
240+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
241+
242+
query T noticetrace
243+
ALTER DATABASE foo RENAME TO foo2
244+
----
245+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
246+
247+
query T noticetrace
248+
CREATE SCHEMA s
249+
----
250+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
251+
252+
query T noticetrace
253+
ALTER SCHEMA s RENAME TO foo
254+
----
255+
NOTICE: setting implicit transaction isolation level to SERIALIZABLE due to schema change
256+
257+
subtest schema_changes_explicit
258+
# Schema changes are prohibited under explicit transactions with weak isolation levels.
153259

154260
statement error transaction involving a schema change needs to be SERIALIZABLE
261+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
155262
ALTER TABLE supermarket ADD COLUMN age INT
156263

264+
statement ok
265+
ROLLBACK
266+
157267
statement error transaction involving a schema change needs to be SERIALIZABLE
268+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
158269
CREATE TABLE foo(a INT)
159270

271+
statement ok
272+
ROLLBACK
273+
160274
statement error transaction involving a schema change needs to be SERIALIZABLE
275+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
161276
DROP TABLE supermarket
162277

278+
statement ok
279+
ROLLBACK
280+
163281
statement error transaction involving a schema change needs to be SERIALIZABLE
282+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
164283
CREATE USER foo
165284

285+
statement ok
286+
ROLLBACK
287+
166288
statement error transaction involving a schema change needs to be SERIALIZABLE
289+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
167290
DROP USER testuser
168291

292+
statement ok
293+
ROLLBACK
294+
169295
statement error transaction involving a schema change needs to be SERIALIZABLE
296+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
170297
GRANT admin TO testuser
171298

299+
statement ok
300+
ROLLBACK
301+
172302
statement error transaction involving a schema change needs to be SERIALIZABLE
303+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
173304
GRANT SELECT ON supermarket TO testuser
174305

306+
statement ok
307+
ROLLBACK
308+
175309
statement error transaction involving a schema change needs to be SERIALIZABLE
310+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
176311
GRANT USAGE ON SCHEMA public TO testuser
177312

313+
statement ok
314+
ROLLBACK
315+
178316
statement error transaction involving a schema change needs to be SERIALIZABLE
317+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
179318
GRANT CONNECT ON DATABASE postgres TO testuser
180319

320+
statement ok
321+
ROLLBACK
322+
181323
statement error transaction involving a schema change needs to be SERIALIZABLE
324+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
182325
CREATE INDEX foo ON supermarket(ends_with, starts_with)
183326

327+
statement ok
328+
ROLLBACK
329+
184330
statement error transaction involving a schema change needs to be SERIALIZABLE
331+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
185332
CREATE FUNCTION f (x INT) RETURNS INT LANGUAGE SQL AS $$
186333
SELECT x+1
187334
$$
188335

189336
statement ok
190-
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
191-
CREATE FUNCTION f (x INT) RETURNS INT LANGUAGE SQL AS $$
192-
SELECT x+1
193-
$$;
194-
COMMIT
337+
ROLLBACK
195338

196339
statement error transaction involving a schema change needs to be SERIALIZABLE
340+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
197341
ALTER FUNCTION f (x INT) RENAME TO g
198342

343+
statement ok
344+
ROLLBACK
345+
199346
statement error transaction involving a schema change needs to be SERIALIZABLE
347+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
200348
GRANT EXECUTE ON FUNCTION f (x INT) TO testuser
201349

350+
statement ok
351+
ROLLBACK
352+
202353
statement error transaction involving a schema change needs to be SERIALIZABLE
354+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
203355
CREATE TYPE typ AS ENUM('a', 'b')
204356

205357
statement ok
206-
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
207-
CREATE TYPE typ AS ENUM('a', 'b');
208-
COMMIT
358+
ROLLBACK
209359

210360
statement error transaction involving a schema change needs to be SERIALIZABLE
361+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
211362
ALTER TYPE typ ADD VALUE 'c'
212363

364+
statement ok
365+
ROLLBACK
366+
213367
statement error transaction involving a schema change needs to be SERIALIZABLE
368+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
214369
GRANT USAGE ON TYPE typ TO testuser
215370

371+
statement ok
372+
ROLLBACK
373+
216374
statement error transaction involving a schema change needs to be SERIALIZABLE
375+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
217376
CREATE DATABASE foo
218377

378+
statement ok
379+
ROLLBACK
380+
219381
statement error transaction involving a schema change needs to be SERIALIZABLE
382+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
220383
ALTER DATABASE postgres RENAME TO foo
221384

385+
statement ok
386+
ROLLBACK
387+
222388
statement error transaction involving a schema change needs to be SERIALIZABLE
389+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
223390
CREATE SCHEMA s
224391

225392
statement ok
226-
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
227-
CREATE SCHEMA s;
228-
COMMIT
393+
ROLLBACK
229394

230395
statement error transaction involving a schema change needs to be SERIALIZABLE
396+
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
231397
ALTER SCHEMA s RENAME TO foo
232398

399+
statement ok
400+
ROLLBACK
401+
233402
subtest end
234403

235404
statement ok

‎pkg/sql/opt/exec/execbuilder/relational.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,6 @@ func (b *Builder) buildRelational(e memo.RelExpr) (execPlan, error) {
167167
var err error
168168

169169
if opt.IsDDLOp(e) {
170-
if b.evalCtx.Txn.IsoLevel().ToleratesWriteSkew() {
171-
return execPlan{}, pgerror.Newf(
172-
pgcode.FeatureNotSupported, "transaction involving a schema change needs to be SERIALIZABLE",
173-
)
174-
}
175170
// Mark the statement as containing DDL for use
176171
// in the SQL executor.
177172
b.IsDDL = true

0 commit comments

Comments
 (0)