Skip to content

Commit

Permalink
sql: session variable to allow multiple modification subqueries of table
Browse files Browse the repository at this point in the history
Add a new session variable, enable_multiple_modifications_of_table,
which can be used instead of sql.multiple_modifications_of_table.enabled
to allow execution of statements with multiple modification subqueries
of the same table.

Instead of making the original cluster setting the GlobalDefault of this
new session setting, the original cluster setting is kept in the
optbuilder logic. This is to avoid breaking applications that are
already toggling the cluster setting mid-session to allow statements.

Fixes: cockroachdb#76261

Release note (sql change): Add a new session variable,
enable_multiple_modifications_of_table, which can be used instead of
cluster variable sql.multiple_modifications_of_table.enabled to allow
statements containing multiple INSERT ON CONFLICT, UPSERT, UPDATE, or
DELETE subqueries modifying the same table. Note that underlying issue
70731 is not fixed. As with sql.multiple_modifications_of_table.enabled,
be warned that with this session variable enabled there is nothing to
prevent the table corruption seen in issue 70731 from occuring if the
same row is modified multiple times by different subqueries of a single
statment. It's best to rewrite these statements, but the session
variable is provided as an aid if this is not possible.

Release justification: low risk, high benefit change to existing
functionality.
  • Loading branch information
michae2 committed May 6, 2022
1 parent c3bc2da commit b8550cf
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 120 deletions.
4 changes: 4 additions & 0 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2976,6 +2976,10 @@ func (m *sessionDataMutator) SetLargeFullScanRows(val float64) {
m.data.LargeFullScanRows = val
}

func (m *sessionDataMutator) SetMultipleModificationsOfTable(val bool) {
m.data.MultipleModificationsOfTable = val
}

// Utility functions related to scrubbing sensitive information on SQL Stats.

// quantizeCounts ensures that the Count field in the
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/information_schema
Original file line number Diff line number Diff line change
Expand Up @@ -4618,6 +4618,7 @@ enable_experimental_alter_column_type_general off
enable_experimental_stream_replication off
enable_implicit_select_for_update on
enable_insert_fast_path on
enable_multiple_modifications_of_table off
enable_multiregion_placement_policy off
enable_seqscan on
enable_zigzag_join on
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
Original file line number Diff line number Diff line change
Expand Up @@ -4025,6 +4025,7 @@ enable_experimental_alter_column_type_general off NULL
enable_experimental_stream_replication off NULL NULL NULL string
enable_implicit_select_for_update on NULL NULL NULL string
enable_insert_fast_path on NULL NULL NULL string
enable_multiple_modifications_of_table off NULL NULL NULL string
enable_multiregion_placement_policy off NULL NULL NULL string
enable_seqscan on NULL NULL NULL string
enable_zigzag_join on NULL NULL NULL string
Expand Down Expand Up @@ -4127,6 +4128,7 @@ enable_experimental_alter_column_type_general off NULL
enable_experimental_stream_replication off NULL user NULL off off
enable_implicit_select_for_update on NULL user NULL on on
enable_insert_fast_path on NULL user NULL on on
enable_multiple_modifications_of_table off NULL user NULL off off
enable_multiregion_placement_policy off NULL user NULL off off
enable_seqscan on NULL user NULL on on
enable_zigzag_join on NULL user NULL on on
Expand Down Expand Up @@ -4225,6 +4227,7 @@ enable_experimental_alter_column_type_general NULL NULL NULL
enable_experimental_stream_replication NULL NULL NULL NULL NULL
enable_implicit_select_for_update NULL NULL NULL NULL NULL
enable_insert_fast_path NULL NULL NULL NULL NULL
enable_multiple_modifications_of_table NULL NULL NULL NULL NULL
enable_multiregion_placement_policy NULL NULL NULL NULL NULL
enable_seqscan NULL NULL NULL NULL NULL
enable_zigzag_join NULL NULL NULL NULL NULL
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/show_source
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ enable_experimental_alter_column_type_general off
enable_experimental_stream_replication off
enable_implicit_select_for_update on
enable_insert_fast_path on
enable_multiple_modifications_of_table off
enable_multiregion_placement_policy off
enable_seqscan on
enable_zigzag_join on
Expand Down
25 changes: 25 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/with
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,31 @@ EXPERIMENTAL SCRUB TABLE t WITH OPTIONS INDEX ALL
statement ok
RESET CLUSTER SETTING sql.multiple_modifications_of_table.enabled

# Multiple mutations can also be explicitly allowed with a session setting.
statement ok
SET enable_multiple_modifications_of_table = true

# Multiple updates of different rows in the same table.
query II rowsort
WITH
u1 AS (UPDATE t SET j = j - 40 WHERE i < 20 RETURNING *),
u2 AS (UPDATE t SET j = j + 40 WHERE i >= 20 RETURNING *)
TABLE u1 UNION ALL TABLE u2
----
2 60
4 60
6 60
8 60
20 440

# Check for corruption.
query TTTTTTTT
EXPERIMENTAL SCRUB TABLE t WITH OPTIONS INDEX ALL
----

statement ok
RESET enable_multiple_modifications_of_table

# Tests with multiple mutations on the same table, modifying the same
# rows. These testcases vary the type of mutation and the form. All should fail
# with an error. When issue 70731 is fixed, some might no longer fail.
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/opt/optbuilder/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cockroachdb/errors"
)

// TODO(michae2): Remove this when #70731 is fixed.
var multipleModificationsOfTableEnabled = settings.RegisterBoolSetting(
"sql.multiple_modifications_of_table.enabled",
"if true, allow statements containing multiple INSERT ON CONFLICT, UPSERT, UPDATE, or DELETE "+
Expand Down Expand Up @@ -491,7 +492,9 @@ func (b *Builder) checkMultipleMutations(tab cat.Table, simpleInsert bool) {
}
allSimpleInserts = allSimpleInserts && simpleInsert
b.areAllTableMutationsSimpleInserts[tab.ID()] = allSimpleInserts
if !allSimpleInserts && !multipleModificationsOfTableEnabled.Get(&b.evalCtx.Settings.SV) {
if !allSimpleInserts &&
!multipleModificationsOfTableEnabled.Get(&b.evalCtx.Settings.SV) &&
!b.evalCtx.SessionData().MultipleModificationsOfTable {
panic(pgerror.Newf(
pgcode.FeatureNotSupported,
"multiple modification subqueries of the same table %q are not supported unless "+
Expand Down
Loading

0 comments on commit b8550cf

Please sign in to comment.