Skip to content

release-22.1: sql/opt: propagate row-level locking mode to index, lookup, inverted, and zigzag joins#81348

Closed
nvb wants to merge 4 commits intocockroachdb:release-22.1from
nvb:backport22.1-79679-60719
Closed

release-22.1: sql/opt: propagate row-level locking mode to index, lookup, inverted, and zigzag joins#81348
nvb wants to merge 4 commits intocockroachdb:release-22.1from
nvb:backport22.1-79679-60719

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented May 17, 2022

Backport:

Please see individual PRs for details.

Release justification: low-risk improvement to rare functionality.

/cc @cockroachdb/release

nvb added 4 commits May 17, 2022 00:18
@nvb
sql/opt: split out row-level locking properties struct
45e1931 
This commit separates an unresolved row-level locking target from a new
row-level locking properties struct. The former is used to represent a
row-level locking clause that is then "resolved" to specific relational
operators. Once attached to a specific operator, the locking mode no
longer needs to include a target, so it makes sense to represent as a
separate struct.

This addresses Radu's comment from cockroachdb#60719.
@nvb
sql/opt: propagate row-level locking mode to index, lookup, and inver…
d26ecf4 
…ted joins

Fixes cockroachdb#56941.

This commit updates the execbuilder to propagate row-level locking modes
through transformations from standard Scan and Join operations to
specialized IndexJoin, LookupJoin, and InvertedJoin operations.

Release note (sql change): table scans performed as a part of index
joins, lookup joins, and inverted joins now respect the row-level
locking strength and wait policy specified by the optional
FOR SHARE/UPDATE [NOWAIT] clause on SELECT statements.
@nvb
sql/opt: propagate row-level locking mode to zigzag joins
0cd1727 
This commit updates the execbuilder to propagate row-level locking modes
through transformations from standard Scan and Join operations to
ZigZagJoins, and allows for the use of zigzag joins when explicit
row-level locking modes are in use.

Release note (sql change): table scans performed as a part of zigzag
joins now respect the row-level locking strength and wait policy
specified by the optional FOR SHARE/UPDATE [NOWAIT] clause on SELECT
statements.
@nvb
sql/opt: add xform testing for select_for_update
7785612 
This commit adds a new `pkg/sql/opt/xform/testdata/rules/select_for_update` test
to `pkg/sql/opt/xform`'s TestRules test suite. This allows the select for update
tests to use the `opt` directive to show joins, assert the use of specific
rules, and assert the propagation of row-level locking modes into joins.
@nvb nvb requested review from a team, michae2 and yuzefovich May 17, 2022 04:35
@nvb nvb requested a review from a team as a code owner May 17, 2022 04:35
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented May 17, 2022 •
edited by nvb
Loading

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented May 17, 2022

This change is Reviewable

@nvb nvb mentioned this pull request May 17, 2022
Merged
yuzefovich
yuzefovich reviewed May 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 27 of 27 files at r1, 19 of 19 files at r2, 15 of 15 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @nvanbenschoten)

pkg/sql/execinfrapb/processors_sql.proto line 492 at r3 (raw file):

  // Indicates the row-level locking strength to be used by the scan. If set to
  // FOR_NONE, no row-level locking should be performed.
  repeated sqlbase.ScanLockingStrength locking_strengths = 1000 [(gogoproto.nullable) = false];

Did you mean to use such large numbers here?

@nvb nvb closed this Dec 28, 2022
@nvb nvb deleted the backport22.1-79679-60719 branch January 3, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yuzefovich yuzefovich yuzefovich left review comments

@michae2 michae2 Awaiting requested review from michae2

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nvb @cockroach-teamcity @yuzefovich