forked from cockroachdb/cockroach
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
sql: do not allow pausable portals on statements containing functions
We currently do not allow pausable portals when a statement may mutate data. However, functions may also mutate data. This change adds a visitor that walks the statements checked by IsAllowedToPause and checks for function expressions. If there is a function expression anywhere in the AST, the statement is not allowed to pause. Users can allow function calls in pausable portals by setting the session setting `enable_functions_in_portals` to true. This setting is false by default. When true, the pausable portal behavior is the same as before this PR. Epic: None Informs: cockroachdb#107130 Release note (sql): This change prevents statements containing function calls from being executed in pausable portals in order to prevent race conditions if the functions modify the database. This behavior is controlled by a session setting called `enable_functions_in_portals`, which is false by default. In order to achieve the old behavior and allow statments containing functions to use portals, users should use `SET enable_functions_in_portals = true`.
- Loading branch information
1 parent
509f8c8
commit c658060
Showing
12 changed files
with
204 additions
and
21 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // Copyright 2023 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package tree_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/sql/parser" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" | ||
| ) | ||
|
|
||
| var pmap = map[bool]string{ | ||
| true: "pausable", | ||
| false: "unpausable", | ||
| } | ||
|
|
||
| func TestIsAllowedToPause(t *testing.T) { | ||
| testData := []struct { | ||
| qry string | ||
| pausable bool | ||
| }{ | ||
| {"SELECT 1;", true}, | ||
| {"WITH cte AS (SELECT * FROM t) SELECT * FROM cte;", true}, | ||
| {"INSERT INTO t VALUES (1);", false}, | ||
| {"WITH cte AS (INSERT INTO t VALUES (1) RETURNING *) SELECT * FROM cte;", false}, | ||
| {"WITH cte AS (SELECT * FROM t) UPDATE t SET v = cte.v FROM cte WHERE t.a = cte.a;", false}, | ||
| {"SELECT f();", false}, | ||
| {"WITH cte AS (SELECT f()) SELECT * FROM cte;", false}, | ||
| {"SELECT * FROM (VALUES (f()));", false}, | ||
| } | ||
|
|
||
| for i, td := range testData { | ||
| t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { | ||
| stmts, err := parser.Parse(td.qry) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if len(stmts) != 1 { | ||
| t.Fatalf("Expected 1 parsed statement, got %d.", len(stmts)) | ||
| } | ||
| if pausable := tree.IsAllowedToPause(stmts[0].AST); pausable != td.pausable { | ||
| t.Errorf("Expected statement \"%s\" to be %s, got %s", td.qry, pmap[td.pausable], pmap[pausable]) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.