Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ability to enable multiple modification subqueries of same table per client session #76261

Closed
dimpavloff opened this issue Feb 8, 2022 · 3 comments · Fixed by #79677
Closed

Ability to enable multiple modification subqueries of same table per client session #76261

dimpavloff opened this issue Feb 8, 2022 · 3 comments · Fixed by #79677
Assignees
@michae2
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community T-sql-queries SQL Queries Team X-blathers-triaged blathers was able to find an owner

Comments

@dimpavloff
Copy link

dimpavloff commented Feb 8, 2022 •
edited by cockroach-jira-scripts
Loading

Is your feature request related to a problem? Please describe.

To mitigate the risks of #70731, #70976 disabled having multiple subquery modifications on the same table in the same transaction and created the sql.multiple_modifications_of_table.enabled cluster setting to re-enable it.

Having it as a cluster setting feels too broad, however. A cluster admin may be forced to enable it for an important query if it is safe to run. However, subsequent new queries may hit the problems of #70731 without anyone noticing.

Describe the solution you'd like
It would be better if sql.multiple_modifications_of_table.enabled can be toggled as a session variable. This allows to locally reason about particular queries.

Jira issue: CRDB-13051
@dimpavloff dimpavloff added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 8, 2022
@blathers-crl
Copy link

blathers-crl bot commented Feb 8, 2022

Hello, I am Blathers. I am here to help you get the issue triaged.

I have CC'd a few people who may be able to assist you:

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Feb 8, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Feb 8, 2022
@michae2
Copy link
Collaborator

michae2 commented Feb 8, 2022

Hi @dimpavloff, this is a good point. I think we were hoping to have a fix to #70731 sooner, but obviously it hasn't happened yet. Let me see what I can do.

@michae2 michae2 self-assigned this Feb 8, 2022
michae2 added a commit to michae2/cockroach that referenced this issue Apr 8, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
46744ff 
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.
@michae2 michae2 mentioned this issue Apr 8, 2022
Merged
michae2 added a commit to michae2/cockroach that referenced this issue Apr 13, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
f55a180 
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.
@craig craig bot closed this as completed in ba9baca Apr 13, 2022
@blathers-crl blathers-crl bot mentioned this issue Apr 13, 2022
Merged
blathers-crl bot pushed a commit that referenced this issue Apr 13, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
7ab5ba8 
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: #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.
@dimpavloff
Copy link
Author

Thanks!

michae2 added a commit that referenced this issue May 4, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
f03888a 
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: #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.
@michae2 michae2 mentioned this issue May 4, 2022
Merged
michae2 added a commit to michae2/cockroach that referenced this issue May 6, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
b8550cf 
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.
michae2 added a commit that referenced this issue May 10, 2022
@michae2
sql: session variable to allow multiple modification subqueries of table
233f92f 
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: #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.
This was referenced Jun 13, 2022
Merged
Open
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michae2 michae2

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community T-sql-queries SQL Queries Team X-blathers-triaged blathers was able to find an owner
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: session variable to allow multiple modification subqueries of table michae2/cockroach
3 participants
@jlinder @dimpavloff @michae2