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.

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

opt: minor fixes for queries that perform locking #141964

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DrewKimball
Copy link
Collaborator

opt: only set volatile for scans that perform locking

This commit makes a minor change to the logical properties for scan
operators. The volatile property is now only set for a scan that takes
locks, rather than any scan with non-default locking.

Epic: None

Release note: None

opt: set volatility for locking operators

This commit sets the volatile property for operators that perform locking,
other than Scan, which was already handled. In most cases there is already
a locking scan below these operators, so there are few plan changes. This
is necessary for a following commit, which will cause Lock operators to
no longer be considered mutation operators (which was previously causing
volatility to be set).

Epic: None

Release note: None

opt: remove Mutation tag from lock operators

This commit removes the Mutation tag from the optimizer lock operator.
As a result, errors that result from checking opt.IsMutationOp or
planFlagContainsMutation will no longer incorrectly occur for plans
that contain lock operators.

Fixes #141961

Release note: None

plpgsql: don't make cursors with locking holdable

This commit changes the close_cursors_at_commit setting to be more
Oracle-compatible. Now, disabling the setting will cause cursors to
remain open after txn commit only for cursors without locking.

This commit also removes the Mutation tag from the optimizer Lock
operator. This is necessary to prevent PL/pgSQL cursors with locking
from returning a must not contain data-modifying statements error
under configurations like read-committed which use lock operators.
Lock operators are equivalent to lookup joins with Locking set, so
the change is correct.

Informs #77101

Release note: None

This commit makes a minor change to the logical properties for scan
operators. The volatile property is now only set for a scan that takes
locks, rather than any scan with non-default locking.

Epic: None

Release note: None
This commit sets the volatile property for operators that perform locking,
other than Scan, which was already handled. In most cases there is already
a locking scan below these operators, so there are few plan changes. This
is necessary for a following commit, which will cause `Lock` operators to
no longer be considered mutation operators (which was previously causing
volatility to be set).

Epic: None

Release note: None
This commit removes the `Mutation` tag from the optimizer lock operator.
As a result, errors that result from checking `opt.IsMutationOp` or
`planFlagContainsMutation` will no longer incorrectly occur for plans
that contain lock operators.

Fixes cockroachdb#141961

Release note: None
This commit changes the `close_cursors_at_commit` setting to be more
Oracle-compatible. Now, disabling the setting will cause cursors to
remain open after txn commit only for cursors without locking.

This commit also removes the `Mutation` tag from the optimizer Lock
operator. This is necessary to prevent PL/pgSQL cursors with locking
from returning a `must not contain data-modifying statements` error
under configurations like read-committed which use lock operators.
Lock operators are equivalent to lookup joins with Locking set, so
the change is correct.

Informs cockroachdb#77101

Release note: None
@DrewKimball DrewKimball requested review from michae2 and a team February 25, 2025 01:39
@DrewKimball DrewKimball requested a review from a team as a code owner February 25, 2025 01:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opt: lock operator is considered a mutation operator
2 participants