opt: fix insert fast path uniqueness check bug#115654
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Dec 6, 2023
Merged
opt: fix insert fast path uniqueness check bug#115654craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
372811d to
3aca545
Compare
rytaft
approved these changes
Dec 5, 2023
Collaborator
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @michae2)
pkg/sql/opt/exec/execbuilder/mutation.go line 202 at r1 (raw file):
// Shadow "i" so that the closure below closes over a local copy of "i", // rather than closing over the incrementing "i". i := i
nit: why not just create a new variable with a different name? or perhaps do something like
uniqueCheck := &ins.UniqueChecks[i]
3aca545 to
5a44f76
Compare
mgartner
commented
Dec 5, 2023
Contributor
Author
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/mutation.go line 202 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: why not just create a new variable with a different name? or perhaps do something like
uniqueCheck := &ins.UniqueChecks[i]
Done.
Contributor
Author
|
TFTR! bors r+ |
Contributor
Author
|
bors r- need to fix commit message. |
Contributor
|
Canceled. |
This commit fixes a bug caused by creating a closure over an incrementing integer in a loop. Fixes cockroachdb#115377 Fixes cockroachdb#115378 Release note (bug fix): A bug has been fixed that caused node crashes and panics when running `INSERT` queries on `REGIONAL BY ROW` tables with `UNIQUE` constraints or indexes. The bug is only present in version 23.2.0-beta.1.
5a44f76 to
3303939
Compare
Contributor
Author
|
bors r+ |
Contributor
|
Build succeeded: |
msbutler
pushed a commit
to msbutler/cockroach
that referenced
this pull request
Dec 6, 2023
115539: roachtest: allocate new cluster when c.Extend fails r=srosenberg a=DarrylWong Previously, failure to extend the lifetime of a cluster would propagate to the worker and end up aborting the entire run. Now, we destroy the cluster and let it allocate a new one. Fixes: cockroachdb#112509 Release note: none Epic: none 115649: cluster-ui: remove leading `/` in settings request r=xinhaoz a=xinhaoz Recently we required that all requests paths from cluster-ui do not start with `/` so that they may be used with any given subpath when constructing proxy requests. We missed removing the prefix from the `settings` api which requests all cluster settings. Release note: None Epic: none ---------------------- Previously, this auto stats enabled label would show disabled even though auto stats collection is enabled. This is because this value comes from the settings request which was not being issued correctly. https://www.loom.com/share/42847f2c63484a508a128c5808df7bdd 115654: opt: fix insert fast path uniqueness check bug r=mgartner a=mgartner This commit fixes a bug caused by creating a closure over an incrementing integer in a loop. Fixes cockroachdb#115377 Fixes cockroachdb#115378 Release note (bug fix): A bug has been fixed that caused node crashes and panics when running `INSERT` queries on `REGIONAL BY ROW` tables with `UNIQUE` constraints or indexes. The bug is only present in version 23.2.0-beta.1. Co-authored-by: DarrylWong <darryl@cockroachlabs.com> Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com> Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug caused by creating a closure over an
incrementing integer in a loop.
Fixes #115377
Fixes #115378
Release note (bug fix): A bug has been fixed that caused node crashes
and panics when running
INSERTqueries onREGIONAL BY ROWtableswith
UNIQUEconstraints or indexes. The bug is only present inversion 23.2.0-beta.1.