Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: disallow schema changes for READ COMMITTED transactions #107216

Merged
merged 1 commit into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# LogicTest: !local-mixed-22.2-23.1

subtest select_for_update

# SELECT FOR UPDATE is prohibited under weaker isolation levels until we improve
# locking. See #57031, #75457, #100144.

Expand Down Expand Up @@ -88,22 +90,27 @@ SELECT aisle + 1 FROM s
statement ok
ROLLBACK

statement ok
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED

# Creating a UDF using SELECT FOR UPDATE should succeed under read committed.
statement ok
CREATE FUNCTION wrangle (name STRING) RETURNS INT LANGUAGE SQL AS $$
SELECT aisle FROM supermarket WHERE person = name FOR UPDATE
$$

statement ok
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED

# But calling that function should fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
INSERT INTO supermarket (person, aisle) VALUES ('grandma', wrangle('matilda'))

statement ok
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE

statement ok
DROP FUNCTION wrangle

statement ok
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED

# Preparing a SELECT FOR UPDATE should succeed under read committed.
statement ok
PREPARE psa AS SELECT aisle FROM supermarket WHERE person = $1::STRING FOR UPDATE
Expand Down Expand Up @@ -138,5 +145,92 @@ SELECT aisle
WHERE starts_with = 'm' AND ends_with = 'lda'
FOR UPDATE

subtest end

subtest schema_changes

# Schema changes are prohibited under weaker isolation levels.

statement error transaction involving a schema change needs to be SERIALIZABLE
ALTER TABLE supermarket ADD COLUMN age INT

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE TABLE foo(a INT)

statement error transaction involving a schema change needs to be SERIALIZABLE
DROP TABLE supermarket

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE USER foo

statement error transaction involving a schema change needs to be SERIALIZABLE
DROP USER testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT admin TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT SELECT ON supermarket TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT USAGE ON SCHEMA public TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT CONNECT ON DATABASE postgres TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE INDEX foo ON supermarket(ends_with, starts_with)

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE FUNCTION f (x INT) RETURNS INT LANGUAGE SQL AS $$
SELECT x+1
$$

statement ok
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
CREATE FUNCTION f (x INT) RETURNS INT LANGUAGE SQL AS $$
SELECT x+1
$$;
COMMIT

statement error transaction involving a schema change needs to be SERIALIZABLE
ALTER FUNCTION f (x INT) RENAME TO g

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT EXECUTE ON FUNCTION f (x INT) TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE TYPE typ AS ENUM('a', 'b')

statement ok
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
CREATE TYPE typ AS ENUM('a', 'b');
COMMIT

statement error transaction involving a schema change needs to be SERIALIZABLE
ALTER TYPE typ ADD VALUE 'c'

statement error transaction involving a schema change needs to be SERIALIZABLE
GRANT USAGE ON TYPE typ TO testuser

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE DATABASE foo

statement error transaction involving a schema change needs to be SERIALIZABLE
ALTER DATABASE postgres RENAME TO foo

statement error transaction involving a schema change needs to be SERIALIZABLE
CREATE SCHEMA s

statement ok
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
CREATE SCHEMA s;
COMMIT

statement error transaction involving a schema change needs to be SERIALIZABLE
ALTER SCHEMA s RENAME TO foo

subtest end

statement ok
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE
14 changes: 7 additions & 7 deletions pkg/sql/logictest/tests/fakedist-disk/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/sql/logictest/tests/fakedist/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/sql/logictest/tests/local-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/sql/logictest/tests/local/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/sql/opt/exec/execbuilder/relational.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ func (b *Builder) buildRelational(e memo.RelExpr) (execPlan, error) {
var err error

if opt.IsDDLOp(e) {
if b.evalCtx.Txn.IsoLevel().ToleratesWriteSkew() {
return execPlan{}, pgerror.Newf(
pgcode.FeatureNotSupported, "transaction involving a schema change needs to be SERIALIZABLE",
)
}
// Mark the statement as containing DDL for use
// in the SQL executor.
b.IsDDL = true
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -6818,7 +6818,7 @@ set_exprs_internal:
It cannot be used by clients. */
SET ROW '(' expr_list ')'
{
$$.val = &tree.SetVar{Values: $4.exprs()}
$$.val = &tree.SetVar{Values: $4.exprs(), SetRow: true}
}

// %Help: SET SESSION - change a session variable
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/set
Original file line number Diff line number Diff line change
Expand Up @@ -626,3 +626,11 @@ SET LOCAL tracing = 'off'
SET LOCAL tracing = ('off') -- fully parenthesized
SET LOCAL tracing = '_' -- literals removed
SET LOCAL tracing = 'off' -- identifiers removed

parse
SET "" = 'a'
----
SET "" = 'a'
SET "" = ('a') -- fully parenthesized
SET "" = '_' -- literals removed
SET "" = 'a' -- identifiers removed
3 changes: 2 additions & 1 deletion pkg/sql/sem/tree/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type SetVar struct {
Values Exprs
Reset bool
ResetAll bool
SetRow bool
}

// Format implements the NodeFormatter interface.
Expand All @@ -47,7 +48,7 @@ func (node *SetVar) Format(ctx *FmtCtx) {
if node.Local {
ctx.WriteString("LOCAL ")
}
if node.Name == "" {
if node.SetRow {
ctx.WriteString("ROW (")
ctx.FormatNode(&node.Values)
ctx.WriteString(")")
Expand Down
Loading