Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
103920: opt: loosen restriction on UDF mutations to the same table r=mgartner a=mgartner

#### opt: loosen restriction on UDF mutations to the same table

To prevent index corruption described in #70731, optbuilder raises an
error when a statement performs multiple mutations to the same table.
This commit loosens this restriction for UDFs that perform mutations
because it is overly strict.

---

The index corruption described in #70731 occurs when a statement
performs multiple writes to the same table. Any reads performed by
successive writes see the snapshot of data as of the beginning of the
statement. They do not read values as of the most recent write within
the same statement. Because these successive writes are based on stale
data, they can write incorrect KVs and cause inconsistencies between
primary and secondary indexes.

Each statement in a UDF body is essentially a child of the statement
that is invoking the UDF. Mutations within UDFs are not as susceptible
to the inconsistencies described above because a UDF with a mutation
must be VOLATILE, and each statement in a VOLATILE UDFs reads at the
latest sequence number. In other words, statements within UDFs can see
previous writes made by any outer statement. This prevents
inconsistencies due to writes based on stale reads. Therefore, the
restriction that prevents multiple writes to the same table can be
lifted in some cases when the writes are performed in UDFs.

However, we cannot forgo restrictions for all writes in UDFs. A parent
statement that calls a UDF cannot be allowed to mutate the same table
that the UDF did. Unlike subsequent statements in the UDF after the
write, the parent statement will not see the UDF's writes, and
inconsistencies could occur.

To define acceptable mutations to the same table within UDFs, we define
a statement tree that represents the hierarchy of statements and
sub-statements in a query. A sub-statement `sub` is any statement within
a UDF. `sub`'s parent is the statement invoking the UDF. Other
statements in the same UDF as `sub` are the `sub`'s siblings. Any
statements in a UDF invoked by `sub` are `sub`'s children. For example,
consider:

    CREATE FUNCTION f1() RETURNS INT LANGUAGE SQL AS 'SELECT 1';
    CREATE FUNCTION f2() RETURNS INT LANGUAGE SQL AS 'SELECT 2 + f3()';
    CREATE FUNCTION f3() RETURNS INT LANGUAGE SQL AS 'SELECT 3';

    SELECT f1(), f2(), f3();

The statement tree for this SELECT would be:

    root: SELECT f1(), f2(), f3()
      ├── f1: SELECT 1
      ├── f2: SELECT 2 + f3()
      │    └── f3: SELECT 3
      └── f3: SELECT 3

We define multiple mutations to the same table as safe if, for every
possible path from the root statement to a leaf statement, either of the
following is true:

  1. There is no more than one mutation to any table.
  2. Or, any table with multiple mutations is modified only by simple
     INSERTs without ON CONFLICT clauses.

As a consequence of this definition, a UDF is now allowed to mutate the
same table as long as it does so in different statements in its body.
Such statements are siblings in the statement tree, and therefore do not
share any path from root to leaf. For example, this is now allowed:

    CREATE FUNCTION ups(a1 INT, a2 INT) RETURNS VOID LANGUAGE SQL AS $$
      UPSERT INTO a VALUES (a1);
      UPSERT INTO a VALUES (a2);
    $$

Similarly, successive invocations of the same UDF that mutates a table
are now allowed:

    CREATE FUNCTION upd(k0 INT, v0 INT) RETURNS VOID LANGUAGE SQL AS $$
      UPDATE kv SET v = v0 WHERE k = k0;
    $$;
    SELECT upd(1, 2), upd(1, 3);

The `statementTree` data structure has been added to enforce this
definition. See its documentation for more details.

Note: These restrictions will likely need to be revisited once we support
recursive UDFs.

Epic: CRDB-25388

Informs #70731

Release note: None


106354: sql: report contention on writes in EXPLAIN ANALYZE r=yuzefovich a=yuzefovich

This commit adds the contention events listener to `planNodeToRowSource` which allows us to add contention time information for mutation planNodes to be shown in EXPLAIN ANALYZE.

Fixes: #106266.

Release note (sql change): CockroachDB now reports contention time encountered while executing mutation statements (INSERT, UPSERT, UPDATE, DELETE) when run via EXPLAIN ANALYZE.

106398: ui: fix infinite re-render on the key visualizer page r=zachlite a=zachlite

Since #101258, the TimeScaleDropdownWithSearchParams can cause infinite re-renders. The exact cause of the bug is not yet diagnosed, but it occurs on the key visualizer page, and seems to be related to the custom duration options. This is tracked in #106395.

This commit removes the dependency on TimeScaleDropdownWithSearchParams from the key visualizer, and replaces it with the vanilla TimeScaleDropdown. The custom duration options are still present.

Informs: #106395
Epic: none
Release note: None

106409: workload: ignore QueryCanceled in random schema change test r=rafiss a=rafiss

fixes #105299

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: zachlite <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
5 people committed Jul 7, 2023
5 parents 8ef0015 + b430742 + 628a1a7 + db1d589 + 99f5df9 commit afe228e
Show file tree
Hide file tree
Showing 22 changed files with 1,003 additions and 135 deletions.
37 changes: 33 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/udf_delete
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,49 @@ query II
SELECT * FROM kv
----

statement error pq: multiple modification subqueries of the same table "kv" are not supported
CREATE FUNCTION f_err(i INT, j INT) RETURNS SETOF RECORD AS
statement ok
CREATE FUNCTION f_kv2(i INT, j INT) RETURNS SETOF RECORD AS
$$
DELETE FROM kv WHERE k = i RETURNING k, v;
DELETE FROM kv WHERE v = j RETURNING k, v;
$$ LANGUAGE SQL;

statement error pq: multiple modification subqueries of the same table "kv" are not supported
CREATE FUNCTION f_err(i INT, j INT) RETURNS SETOF RECORD AS
statement ok
INSERT INTO kv VALUES (1, 2), (3, 4), (5, 6), (7, 8)

statement ok
SELECT f_kv2(1, 9)

query II rowsort
SELECT * FROM kv
----
3 4
5 6
7 8

statement ok
SELECT f_kv2(i, j) FROM (VALUES (3, 0), (0, 8)) v(i, j)

query II
SELECT * FROM kv
----
5 6

statement ok
CREATE FUNCTION f_no_op(i INT, j INT) RETURNS SETOF RECORD AS
$$
INSERT INTO kv VALUES (i, j);
DELETE FROM kv WHERE k=i AND v=j RETURNING k, v;
$$ LANGUAGE SQL;

statement ok
SELECT f_no_op(9, 10)

query II
SELECT * FROM kv
----
5 6

statement ok
CREATE TABLE unindexed (
k INT PRIMARY KEY,
Expand Down
12 changes: 7 additions & 5 deletions pkg/sql/logictest/testdata/logic_test/udf_update
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ SELECT f2(5,32);
----
(5,32)

statement error pq: multiple modification subqueries of the same table \"t\" are not supported
query TT
SELECT f2(5,9), f2(7,11);
----
(5,9) (7,11)

query T nosort
SELECT f2(x,y) FROM (VALUES (1,16),(1,17)) v(x,y);
Expand All @@ -80,10 +82,10 @@ SELECT f2(x,y) FROM (VALUES (1,16),(1,17)) v(x,y);
query II rowsort
SELECT * FROM t;
----
1 17
3 4
5 32
7 8
1 17
3 4
5 9
7 11

statement ok
CREATE TABLE t2 (a INT, b INT, c INT);
Expand Down
55 changes: 40 additions & 15 deletions pkg/sql/logictest/testdata/logic_test/udf_upsert
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,27 @@ SELECT f_ocdn(1,2,1);
----
NULL

statement error pq: multiple modification subqueries of the same table \"t_ocdn\" are not supported
query TTTT
SELECT f_ocdn(1,1,1), f_ocdn(3,2,2), f_ocdn(6,6,2), f_ocdn(2,1,1);
----
NULL (3,2,2) (6,6,2) NULL

query T nosort
SELECT f_ocdn(x, y, z) FROM (VALUES (1, 1, 1), (2, 2, 1), (3, 3, 3), (3, 4, 4)) v(x, y, z)
SELECT f_ocdn(x, y, z) FROM (VALUES (1, 1, 1), (2, 2, 1), (3, 3, 3), (3, 4, 4), (5, 5, 5)) v(x, y, z)
----
NULL
(2,2,1)
(3,3,3)
NULL
NULL
NULL
(5,5,5)

query III rowsort
SELECT * FROM t_ocdn
----
1 1 1
2 2 1
3 3 3
1 1 1
3 2 2
5 5 5
6 6 2


statement ok
Expand All @@ -55,24 +59,45 @@ $$
$$ LANGUAGE SQL;

statement ok
SELECT f_ocdn_2vals(5,5,5,5,5,5);
SELECT f_ocdn_2vals(7,7,7,7,7,7);

query III rowsort
SELECT * FROM t_ocdn;
----
1 1 1
2 2 1
3 3 3
5 5 5
1 1 1
3 2 2
5 5 5
6 6 2
7 7 7

statement error pq: multiple modification subqueries of the same table \"t_ocdn\" are not supported
CREATE FUNCTION f_err(i INT, j INT, k INT, m INT, n INT, o INT) RETURNS RECORD AS
statement ok
CREATE FUNCTION f_multi_ins(i INT, j INT, k INT, m INT, n INT, o INT) RETURNS RECORD AS
$$
INSERT INTO t_ocdn VALUES (i, j, k) ON CONFLICT DO NOTHING;
INSERT INTO t_ocdn VALUES (m, n, o) ON CONFLICT DO NOTHING;
SELECT * FROM t_ocdn WHERE t.a=i OR t.a=m;
SELECT * FROM t_ocdn WHERE a=i OR a=m ORDER BY a;
$$ LANGUAGE SQL;

query TT
SELECT f_multi_ins(1, 1, 1, 1, 1, 1), f_multi_ins(1, 1, 1, 1, 1, 1)
----
(1,1,1) (1,1,1)

query TT
SELECT f_multi_ins(2, 2, 2, 3, 3, 3), f_multi_ins(3, 3, 3, 4, 4, 4)
----
(3,2,2) (3,2,2)

query III rowsort
SELECT * FROM t_ocdn
----
1 1 1
3 2 2
4 4 4
5 5 5
6 6 2
7 7 7

subtest on_conflict_do_update

statement ok
Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/logictest/testdata/logic_test/with
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ SELECT i, j FROM u@u_j_idx

# Multiple upserts of the same row. Might succeed after issue 70731 is fixed,
# depending on the implementation, but should not cause corruption.
query error pgcode 0A000 multiple modification subqueries of the same table
query error pgcode 0A000 multiple mutations of the same table
WITH
v AS (UPSERT INTO u VALUES (0, 1) RETURNING *),
w AS (UPSERT INTO u SELECT i, j + 1 FROM v RETURNING *)
Expand All @@ -1071,7 +1071,7 @@ SELECT i, j FROM u@u_j_idx
# Multiple updates of the same row. Might succeed after issue 70731 is fixed,
# depending on the implementation, but should not cause corruption. The order of
# CTE execution is not necessarily defined here.
query error pgcode 0A000 multiple modification subqueries of the same table
query error pgcode 0A000 multiple mutations of the same table
WITH
v AS (UPDATE u SET j = 3 WHERE i = 0 RETURNING *),
w AS (UPDATE u SET j = 4 WHERE i = 0 RETURNING *)
Expand All @@ -1090,7 +1090,7 @@ SELECT i, j FROM u@u_j_idx
# Multiple updates of the same row. Might succeed after issue 70731 is fixed,
# depending on the implementation, but should not cause corruption. The order of
# CTE execution should be deterministic.
query error pgcode 0A000 multiple modification subqueries of the same table
query error pgcode 0A000 multiple mutations of the same table
WITH
v AS (UPDATE u SET j = 5 WHERE i = 0 RETURNING *),
w AS (UPDATE u SET j = v.j + 1 FROM v WHERE u.i = v.i RETURNING *)
Expand All @@ -1108,7 +1108,7 @@ SELECT i, j FROM u@u_j_idx

# Multiple inserts of the same row into the same table, most should become
# updates due to conflicts. Might succeed after issue 70731 is fixed.
query error pgcode 0A000 multiple modification subqueries of the same table
query error pgcode 0A000 multiple mutations of the same table
WITH
v AS (INSERT INTO u VALUES (0, 42), (1, 42) ON CONFLICT (i) DO UPDATE SET j = 52 RETURNING *)
INSERT INTO u SELECT i, j + 1 FROM v ON CONFLICT (i) DO UPDATE SET j = v.j + 100 RETURNING *
Expand All @@ -1125,7 +1125,7 @@ SELECT i, j FROM u@u_j_idx

# Multiple deletes of the same row. Might succeed after issue 70731 is fixed,
# though the order of CTE execution is undefined.
query error pgcode 0A000 multiple modification subqueries of the same table
query error pgcode 0A000 multiple mutations of the same table
WITH
v AS (DELETE FROM u ORDER BY i LIMIT 1 RETURNING *),
w AS (DELETE FROM u ORDER BY i LIMIT 2 RETURNING *)
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/opt/exec/explain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ go_test(
embed = [":explain"],
deps = [
"//pkg/base",
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/sql/catalog/colinfo",
"//pkg/sql/execinfra",
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opt/exec/explain/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ import (
"os"
"testing"

"github.com/cockroachdb/cockroach/pkg/security/securityassets"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
)

func TestMain(m *testing.M) {
securityassets.SetLoader(securitytest.EmbeddedAssets)
serverutils.InitTestServerFactory(server.TestServerFactory)
os.Exit(m.Run())
}
Loading

0 comments on commit afe228e

Please sign in to comment.