opt: add implicit SELECT FOR UPDATE to initial scan of DELETE

This commit makes it so that we apply implicit SELECT FOR UPDATE locking behavior to the initial scan of the DELETE operation in some cases. Namely, we do so when the input to the DELETE is either a scan or an index join on top of a scan (with any number of renders on top) - these conditions mean that all filters were pushed down into the scan, so we won't lock any unnecessary rows. I think it's only possible to have at most one render expression on top of the scan, but I chose to be defensive and allowed nested renders too. In such form the conditions are exactly the same as we use for adding SFU to UPDATEs. Existing `enable_implicit_select_for_update` session variable is consulted. Release note (sql change): DELETE statements now acquire locks using the FOR UPDATE locking mode during their initial row scan in some comes, which improves performance for contended workloads. This behavior is configurable using the `enable_implicit_select_for_update` session variable.
cockroachdb · Dec 10, 2024 · 8935b1f · 8935b1f
1 parent a247ae5
commit 8935b1f
Show file tree

Hide file tree

Showing 14 changed files with 67 additions and 25 deletions.
diff --git a/pkg/sql/opt/exec/execbuilder/mutation.go b/pkg/sql/opt/exec/execbuilder/mutation.go
@@ -1131,6 +1131,10 @@ var forUpdateLocking = opt.Locking{
 func (b *Builder) shouldApplyImplicitLockingToMutationInput(
 	mutExpr memo.RelExpr,
 ) (opt.TableID, error) {
+	if !b.evalCtx.SessionData().ImplicitSelectForUpdate {
+		return 0, nil
+	}
+
 	switch t := mutExpr.(type) {
 	case *memo.InsertExpr:
 		// Unlike with the other three mutation expressions, it never makes
@@ -1177,10 +1181,6 @@ func (b *Builder) shouldApplyImplicitLockingToMutationInput(
 // not worth risking the transformation being a pessimization, so it is only
 // applied when doing so does not risk creating artificial contention.
 func (b *Builder) shouldApplyImplicitLockingToUpdateInput(upd *memo.UpdateExpr) opt.TableID {
-	if !b.evalCtx.SessionData().ImplicitSelectForUpdate {
-		return 0
-	}
-
 	// Try to match the Update's input expression against the pattern:
 	//
 	//   [Project]* [IndexJoin] Scan
@@ -1201,10 +1201,6 @@ func (b *Builder) shouldApplyImplicitLockingToUpdateInput(upd *memo.UpdateExpr)
 // an UPSERT statement. If the builder should lock the initial row scan, it
 // returns the TableID of the scan, otherwise it returns 0.
 func (b *Builder) shouldApplyImplicitLockingToUpsertInput(ups *memo.UpsertExpr) opt.TableID {
-	if !b.evalCtx.SessionData().ImplicitSelectForUpdate {
-		return 0
-	}
-
 	// Try to match the Upsert's input expression against the pattern:
 	//
 	//   [Project]* (LeftJoin Scan | LookupJoin) [Project]* Values
@@ -1239,10 +1235,19 @@ func (b *Builder) shouldApplyImplicitLockingToUpsertInput(ups *memo.UpsertExpr)
 // should apply a FOR UPDATE row-level locking mode to the initial row scan of a
 // DELETE statement. If the builder should lock the initial row scan, it returns
 // the TableID of the scan, otherwise it returns 0.
-//
-// TODO(nvanbenschoten): implement this method to match on appropriate Delete
-// expression trees and apply a row-level locking mode.
 func (b *Builder) shouldApplyImplicitLockingToDeleteInput(del *memo.DeleteExpr) opt.TableID {
+	// Try to match the Delete's input expression against the pattern:
+	//
+	//   [Project]* [IndexJoin] Scan
+	//
+	input := del.Input
+	input = unwrapProjectExprs(input)
+	if idxJoin, ok := input.(*memo.IndexJoinExpr); ok {
+		input = idxJoin.Input
+	}
+	if scan, ok := input.(*memo.ScanExpr); ok {
+		return scan.Table
+	}
 	return 0
 }
 

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/cascade b/pkg/sql/opt/exec/execbuilder/testdata/cascade
@@ -900,6 +900,7 @@ vectorized: true
 │             missing stats
 │             table: self@self_pkey
 │             spans: [/4 - /4]
+│             locking strength: for update
 │
 └── • fk-cascade
     │ fk: self_b_fkey
@@ -984,6 +985,7 @@ vectorized: true
 │             missing stats
 │             table: loop_a@loop_a_pkey
 │             spans: [/'loop_a-pk1' - /'loop_a-pk1']
+│             locking strength: for update
 │
 └── • fk-cascade
     │ fk: loop_b_cascade_delete_fkey
@@ -1085,6 +1087,7 @@ quality of service: regular
 │             missing stats
 │             table: loop_a@loop_a_pkey
 │             spans: [/'loop_a-pk1' - /'loop_a-pk1']
+│             locking strength: for update
 │
 └── • fk-cascade
     │ fk: loop_b_cascade_delete_fkey
@@ -1235,6 +1238,7 @@ vectorized: true
 │             missing stats
 │             table: t3@t3_pkey
 │             spans: [/1 - /1]
+│             locking strength: for update
 │
 ├── • fk-cascade
 │   │ fk: fk1

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/delete b/pkg/sql/opt/exec/execbuilder/testdata/delete
@@ -141,6 +141,7 @@ vectorized: true
       table: unindexed@unindexed_pkey
       spans: [/1 - ]
       limit: 1
+      locking strength: for update
 
 query T
 EXPLAIN DELETE FROM indexed WHERE value = 5 LIMIT 10
@@ -157,6 +158,7 @@ vectorized: true
       table: indexed@indexed_value_idx
       spans: [/5 - /5]
       limit: 10
+      locking strength: for update
 
 query T
 EXPLAIN DELETE FROM indexed LIMIT 10
@@ -173,6 +175,7 @@ vectorized: true
       table: indexed@indexed_value_idx
       spans: LIMITED SCAN
       limit: 10
+      locking strength: for update
 
 # TODO(andyk): Prune columns so that index-join is not necessary.
 query T
@@ -190,6 +193,7 @@ vectorized: true
       table: indexed@indexed_value_idx
       spans: [/5 - /5]
       limit: 10
+      locking strength: for update
 
 # Ensure that index hints in DELETE statements force the choice of a specific index
 # as described in #38799.
@@ -213,6 +217,7 @@ vectorized: true
       estimated row count: 1,000 (missing stats)
       table: t38799@foo
       spans: FULL SCAN
+      locking strength: for update
 
 # Tracing tests for fast delete.
 statement ok
@@ -331,12 +336,14 @@ vectorized: true
         │ estimated row count: 990 (missing stats)
         │ table: xyz@xyz_pkey
         │ key columns: x
+        │ locking strength: for update
         │
         └── • scan
               columns: (x, y)
               estimated row count: 990 (missing stats)
               table: xyz@xyz_y_idx
               spans: /1-/1000 /2001-/3000
+              locking strength: for update
 
 # Testcase for issue 105803.
 

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain_redact b/pkg/sql/opt/exec/execbuilder/testdata/explain_redact
@@ -1458,6 +1458,7 @@ vectorized: true
           missing stats
           table: f@f_f_idx (partial index)
           spans: 1 span
+          locking strength: for update
 
 query T
 EXPLAIN (VERBOSE, REDACT) DELETE FROM f WHERE f = 8.5
@@ -1482,6 +1483,7 @@ vectorized: true
           estimated row count: 10 (missing stats)
           table: f@f_f_idx (partial index)
           spans: 1 span
+          locking strength: for update
 
 query T
 EXPLAIN (OPT, REDACT) DELETE FROM f WHERE f = 8.5

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/fk b/pkg/sql/opt/exec/execbuilder/testdata/fk
@@ -418,6 +418,7 @@ vectorized: true
 │             missing stats
 │             table: parent@parent_pkey
 │             spans: [/3 - /3]
+│             locking strength: for update
 │
 ├── • constraint-check
 │   │
@@ -464,6 +465,7 @@ vectorized: true
 │             missing stats
 │             table: parent@parent_pkey
 │             spans: [/3 - /3]
+│             locking strength: for update
 │
 ├── • constraint-check
 │   │
@@ -511,6 +513,7 @@ vectorized: true
 │             missing stats
 │             table: parent@parent_pkey
 │             spans: [/3 - /3]
+│             locking strength: for update
 │
 ├── • constraint-check
 │   │
@@ -575,6 +578,7 @@ vectorized: true
 │             missing stats
 │             table: doubleparent@doubleparent_pkey
 │             spans: [/10 - /10]
+│             locking strength: for update
 │
 └── • constraint-check
     │
@@ -1982,6 +1986,7 @@ vectorized: true
 │             estimated row count: 1 (100% of the table; stats collected <hidden> ago)
 │             table: p@p_pkey
 │             spans: [/1 - ]
+│             locking strength: for update
 │
 └── • constraint-check
     │
@@ -2071,6 +2076,7 @@ vectorized: true
 │             estimated row count: 1 (100% of the table; stats collected <hidden> ago)
 │             table: p@p_pkey
 │             spans: [/1 - ]
+│             locking strength: for update
 │
 └── • constraint-check
     │
@@ -2181,6 +2187,7 @@ vectorized: true
 │             missing stats
 │             table: partial_parent@partial_parent_pkey
 │             spans: [/1 - /1]
+│             locking strength: for update
 │
 └── • constraint-check
     │

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/inverted_index b/pkg/sql/opt/exec/execbuilder/testdata/inverted_index
@@ -49,7 +49,7 @@ InitPut /Table/106/2/Arr/7/1/0 -> /BYTES/
 query T kvtrace
 DELETE FROM d WHERE a=1
 ----
-Scan /Table/106/1/1/0
+Scan /Table/106/1/1/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/2/Arr/0/1/0
 Del /Table/106/2/Arr/7/1/0
 Del /Table/106/1/1/0
@@ -96,7 +96,7 @@ Del /Table/106/2/Arr/1/4/0
 query T kvtrace
 DELETE FROM d WHERE a=4
 ----
-Scan /Table/106/1/4/0
+Scan /Table/106/1/4/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/1/4/0
 
 # Tests for array inverted indexes.

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/inverted_index_multi_column b/pkg/sql/opt/exec/execbuilder/testdata/inverted_index_multi_column
@@ -65,7 +65,7 @@ InitPut /Table/106/3/333/"foo"/Arr/"a"/"b"/3/0 -> /BYTES/
 query T kvtrace
 DELETE FROM t WHERE k = 2
 ----
-Scan /Table/106/1/2/0
+Scan /Table/106/1/2/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/2/333/Arr/0/2/0
 Del /Table/106/2/333/Arr/7/2/0
 Del /Table/106/3/333/"foo"/Arr/0/2/0
@@ -75,7 +75,7 @@ Del /Table/106/1/2/0
 query T kvtrace
 DELETE FROM t WHERE k = 3
 ----
-Scan /Table/106/1/3/0
+Scan /Table/106/1/3/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/2/333/Arr/3/3/0
 Del /Table/106/2/333/Arr/"a"/"b"/3/0
 Del /Table/106/3/333/"foo"/Arr/3/3/0
@@ -110,7 +110,7 @@ Del /Table/106/3/333/"foo"/Arr/1/4/0
 query T kvtrace
 DELETE FROM t WHERE k = 4
 ----
-Scan /Table/106/1/4/0
+Scan /Table/106/1/4/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/1/4/0
 
 # Insert NULL non-inverted value.
@@ -147,7 +147,7 @@ InitPut /Table/106/3/NULL/"foo"/"a"/"b"/5/0 -> /BYTES/
 query T kvtrace
 DELETE FROM t WHERE k = 5
 ----
-Scan /Table/106/1/5/0
+Scan /Table/106/1/5/0 lock Exclusive (Block, Unreplicated)
 Del /Table/106/2/NULL/"a"/"b"/5/0
 Del /Table/106/3/NULL/"foo"/"a"/"b"/5/0
 Del /Table/106/1/5/0
diff --git a/pkg/sql/opt/exec/execbuilder/testdata/not_visible_index b/pkg/sql/opt/exec/execbuilder/testdata/not_visible_index
@@ -853,6 +853,7 @@ vectorized: true
 │             missing stats
 │             table: parent@parent_pkey
 │             spans: [/2 - /2]
+│             locking strength: for update
 │
 └── • constraint-check
     │
@@ -1043,6 +1044,7 @@ vectorized: true
 │             missing stats
 │             table: parent@parent_pkey
 │             spans: [/2 - /2]
+│             locking strength: for update
 │
 ├── • fk-cascade
 │   │ fk: child_delete_p_fkey
@@ -1054,6 +1056,7 @@ vectorized: true
 │             missing stats
 │             table: child_delete@c_delete_idx_invisible
 │             spans: [/2 - /2]
+│             locking strength: for update
 │
 └── • constraint-check
     │

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/orderby b/pkg/sql/opt/exec/execbuilder/testdata/orderby
@@ -736,6 +736,7 @@ vectorized: true
           estimated row count: 1 (missing stats)
           table: t@t_pkey
           spans: /3/0
+          locking strength: for update
 
 query T
 EXPLAIN (VERBOSE) UPDATE t SET c = TRUE RETURNING b

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/partial_index b/pkg/sql/opt/exec/execbuilder/testdata/partial_index
@@ -265,15 +265,15 @@ InitPut /Table/113/2/3/2/0 -> /BYTES/
 query T kvtrace
 DELETE FROM t WHERE a = 5
 ----
-Scan /Table/112/1/5/0
+Scan /Table/112/1/5/0 lock Exclusive (Block, Unreplicated)
 Del /Table/112/2/4/5/0
 Del /Table/112/1/5/0
 
 # Deleted row matches the first partial index.
 query T kvtrace
 DELETE FROM t WHERE a = 6
 ----
-Scan /Table/112/1/6/0
+Scan /Table/112/1/6/0 lock Exclusive (Block, Unreplicated)
 Del /Table/112/2/11/6/0
 Del /Table/112/3/11/6/0
 Del /Table/112/1/6/0
@@ -282,7 +282,7 @@ Del /Table/112/1/6/0
 query T kvtrace
 DELETE FROM t WHERE a = 12
 ----
-Scan /Table/112/1/12/0
+Scan /Table/112/1/12/0 lock Exclusive (Block, Unreplicated)
 Del /Table/112/2/11/12/0
 Del /Table/112/3/11/12/0
 Del /Table/112/4/"foo"/12/0
@@ -293,15 +293,15 @@ Del /Table/112/1/12/0
 query T kvtrace
 DELETE FROM u WHERE k = 1
 ----
-Scan /Table/113/1/1/0
+Scan /Table/113/1/1/0 lock Exclusive (Block, Unreplicated)
 Del /Table/113/1/1/0
 
 # Deleted row matches partial index with predicate column that is not
 # indexed.
 query T kvtrace
 DELETE FROM u WHERE k = 2
 ----
-Scan /Table/113/1/2/0
+Scan /Table/113/1/2/0 lock Exclusive (Block, Unreplicated)
 Del /Table/113/2/3/2/0
 Del /Table/113/1/2/0
 
@@ -890,15 +890,15 @@ CPut /Table/107/1/2/0 -> /TUPLE/
 query T kvtrace
 DELETE FROM inv WHERE a = 1
 ----
-Scan /Table/107/1/1/0
+Scan /Table/107/1/1/0 lock Exclusive (Block, Unreplicated)
 Del /Table/107/2/"num"/1/1/0
 Del /Table/107/2/"x"/"y"/1/0
 Del /Table/107/1/1/0
 
 query T kvtrace
 DELETE FROM inv WHERE a = 2
 ----
-Scan /Table/107/1/2/0
+Scan /Table/107/1/2/0 lock Exclusive (Block, Unreplicated)
 Del /Table/107/1/2/0
 
 # ---------------------------------------------------------

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/show_trace b/pkg/sql/opt/exec/execbuilder/testdata/show_trace
@@ -177,7 +177,7 @@ SET tracing = off
 query TT
 $trace_query
 ----
-colbatchscan  Scan /Table/108/{1-2}
+colbatchscan  Scan /Table/108/{1-2} lock Exclusive (Block, Unreplicated)
 colbatchscan  fetched: /kv/kv_pkey/1/v -> /2
 count         Del /Table/108/2/2/0
 count         Del /Table/108/1/1/0

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/spool b/pkg/sql/opt/exec/execbuilder/testdata/spool
@@ -68,6 +68,7 @@ vectorized: true
                   missing stats
                   table: t@t_pkey
                   spans: FULL SCAN
+                  locking strength: for update
 
 
 query T
@@ -196,6 +197,7 @@ vectorized: true
                   missing stats
                   table: t@t_pkey
                   spans: FULL SCAN
+                  locking strength: for update
 
 query T
 EXPLAIN SELECT * FROM [UPDATE t SET x = x + 1 RETURNING x] LIMIT 1