opt: add update checks for unique constraints #57930
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @RaduBerinde, and @rytaft)
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 42 at r1 (raw file):
// a check. if mb.tab.Unique(i).WithoutIndex() && h.init(mb, i) { mb.uniqueChecks = append(mb.uniqueChecks, h.buildInsertionCheck())
[nit] maybe a comment why the insertion check just works
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 48 at r1 (raw file):
} func (mb *mutationBuilder) buildUniqueChecksForUpdate() {
[nit] worth adding a comment, and mention that this builds checks for "unique without index" constraints
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 75 at r1 (raw file):
// UNIQUE WITHOUT INDEX constraints on the table. Also returns the ordinal of // the first matching constraint if one exists. func (mb *mutationBuilder) hasUniqueWithoutIndexConstraints() (ord int, ok bool) {
I don't think returning the ordinal is worthwhile here. It's a minuscule optimization, this stuff pales in comparison with the total cost of the actual uniqueness check.
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 77 at r1 (raw file):
func (mb *mutationBuilder) hasUniqueWithoutIndexConstraints() (ord int, ok bool) { uniqueCount := mb.tab.UniqueCount() if uniqueCount == 0 {
[nit] We shouldn't need this special code path
This commit adds checks for unique constraints when planning updates in the optimizer. This does not yet impact anything outside of the optimizer tests, since UNIQUE WITHOUT INDEX is still not fully supported outside of the optimizer test catalog. Informs cockroachdb#41535 Release note: None
rytaft
force-pushed
the
update-checks
branch
from
2eddd70 to
20b524a
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 42 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] maybe a comment why the insertion check just works
Did you mean to add this to the update code below? I added a comment there (let me know if that's not what you had in mind).
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 48 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] worth adding a comment, and mention that this builds checks for "unique without index" constraints
Done.
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 75 at r1 (raw file):
Previously, RaduBerinde wrote…
I don't think returning the ordinal is worthwhile here. It's a minuscule optimization, this stuff pales in comparison with the total cost of the actual uniqueness check.
Done.
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 77 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] We shouldn't need this special code path
Done.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/mutation_builder_unique.go, line 42 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
Did you mean to add this to the update code below? I added a comment there (let me know if that's not what you had in mind).
Yeah, looks good, I pointed to the wrong section
|
Build succeeded: |
This commit adds checks for unique constraints when planning updates
in the optimizer. This does not yet impact anything outside of the optimizer
tests, since
UNIQUE WITHOUT INDEXis still not fully supported outside of theoptimizer test catalog.
Informs #41535
Release note: None