From 8935b1f19c5a99efc83227bd4410f1ac452daba4 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Mon, 9 Dec 2024 17:13:46 -0800 Subject: [PATCH] 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. --- pkg/sql/opt/exec/execbuilder/mutation.go | 27 +++++++++++-------- pkg/sql/opt/exec/execbuilder/testdata/cascade | 4 +++ pkg/sql/opt/exec/execbuilder/testdata/delete | 7 +++++ .../exec/execbuilder/testdata/explain_redact | 2 ++ pkg/sql/opt/exec/execbuilder/testdata/fk | 7 +++++ .../exec/execbuilder/testdata/inverted_index | 4 +-- .../testdata/inverted_index_multi_column | 8 +++--- .../execbuilder/testdata/not_visible_index | 3 +++ pkg/sql/opt/exec/execbuilder/testdata/orderby | 1 + .../exec/execbuilder/testdata/partial_index | 14 +++++----- .../opt/exec/execbuilder/testdata/show_trace | 2 +- pkg/sql/opt/exec/execbuilder/testdata/spool | 2 ++ .../testdata/sql_activity_stats_compaction | 4 +++ .../exec/execbuilder/testdata/virtual_columns | 7 +++++ 14 files changed, 67 insertions(+), 25 deletions(-) diff --git a/pkg/sql/opt/exec/execbuilder/mutation.go b/pkg/sql/opt/exec/execbuilder/mutation.go index cab0fcd2afe9..c16b7f445fbf 100644 --- 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 index 9d275e9e42f4..23ca5d3178d8 100644 --- 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 index 845438ed6996..a6724af86c45 100644 --- 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 index d9bdf3b233c3..8291ed8aa126 100644 --- 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 index ca3a775c95ee..06af7e7c434c 100644 --- 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 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 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 index 755a2b10e0bd..2acb5310c1a2 100644 --- 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 index 78be7fade70c..36e2db63f394 100644 --- 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 index 041359667359..429918cd8698 100644 --- 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 index 254189ed9f4c..91e554740116 100644 --- 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 index eb0c74182812..eaa0a1673c5b 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/partial_index +++ b/pkg/sql/opt/exec/execbuilder/testdata/partial_index @@ -265,7 +265,7 @@ 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 @@ -273,7 +273,7 @@ Del /Table/112/1/5/0 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,7 +293,7 @@ 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 @@ -301,7 +301,7 @@ Del /Table/113/1/1/0 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,7 +890,7 @@ 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 @@ -898,7 +898,7 @@ 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 index 16e8d7582a3c..552d2e4f429f 100644 --- 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 index 2b77d65a56e5..56b58cc8b437 100644 --- 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 diff --git a/pkg/sql/opt/exec/execbuilder/testdata/sql_activity_stats_compaction b/pkg/sql/opt/exec/execbuilder/testdata/sql_activity_stats_compaction index ec1a1118fcd0..86dfa8f016ca 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/sql_activity_stats_compaction +++ b/pkg/sql/opt/exec/execbuilder/testdata/sql_activity_stats_compaction @@ -264,6 +264,7 @@ vectorized: true table: statement_statistics@primary spans: /0-/0/2022-05-04T15:59:59.999999001Z limit: 1024 + locking strength: for update statement ok ALTER TABLE system.transaction_statistics INJECT STATISTICS '[ @@ -448,6 +449,7 @@ vectorized: true table: transaction_statistics@primary spans: /0-/0/2022-05-04T15:59:59.999999001Z limit: 1024 + locking strength: for update query T EXPLAIN (VERBOSE) @@ -507,6 +509,7 @@ vectorized: true table: statement_statistics@primary spans: /0/2022-05-04T14:00:00Z/"123"/"234"/"345"/"test"/1-/0/2022-05-04T15:59:59.999999001Z limit: 1024 + locking strength: for update query T EXPLAIN (VERBOSE) @@ -561,6 +564,7 @@ vectorized: true table: transaction_statistics@primary spans: /0/2022-05-04T14:00:00Z/"123"/"test"/2-/0/2022-05-04T15:59:59.999999001Z limit: 1024 + locking strength: for update statement ok RESET CLUSTER SETTING sql.stats.flush.interval diff --git a/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns b/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns index ff32313e4987..daf585017889 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns +++ b/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns @@ -286,6 +286,7 @@ vectorized: true estimated row count: 333 (missing stats) table: t@t_pkey spans: /2- + locking strength: for update query T EXPLAIN (VERBOSE) DELETE FROM t WHERE v = 1 @@ -336,6 +337,7 @@ vectorized: true estimated row count: 333 (missing stats) table: t_idx@t_idx_pkey spans: /2- + locking strength: for update query T EXPLAIN (VERBOSE) DELETE FROM t_idx WHERE a > 1 RETURNING v @@ -363,6 +365,7 @@ vectorized: true estimated row count: 333 (missing stats) table: t_idx@t_idx_pkey spans: /2- + locking strength: for update query T EXPLAIN (VERBOSE) DELETE FROM t_idx WHERE v = 1 @@ -387,12 +390,14 @@ vectorized: true │ estimated row count: 333 (missing stats) │ table: t_idx@t_idx_pkey │ key columns: a + │ locking strength: for update │ └── • scan columns: (a) estimated row count: 10 (missing stats) table: t_idx@t_idx_v_idx spans: /1-/2 + locking strength: for update query T EXPLAIN (VERBOSE) DELETE FROM t_idx WHERE a + b = 1 @@ -417,12 +422,14 @@ vectorized: true │ estimated row count: 333 (missing stats) │ table: t_idx@t_idx_pkey │ key columns: a + │ locking strength: for update │ └── • scan columns: (a) estimated row count: 10 (missing stats) table: t_idx@t_idx_v_idx spans: /1-/2 + locking strength: for update subtest Update