-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
sql: ignore shard column during unique constraint searching #74140
sql: ignore shard column during unique constraint searching #74140
Conversation
1dff2d8
to
60b2a8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @chengxiong-ruan and @postamar)
pkg/sql/catalog/descpb/index.go, line 86 at r1 (raw file):
// key constraint with the provided set of referencedColumnIDs. func (desc *IndexDescriptor) IsValidReferencedUniqueConstraint(referencedColIDs ColumnIDs) bool { colIDs := desc.ExplicitColumnIDsWithoutShardColumn()
why not just invoke this directly in the last part of the condition and short-circuit it most of the time. Also, why export it?
pkg/sql/catalog/descpb/index.go, line 86 at r1 (raw file): Previously, ajwerner wrote…
Just wanted to make the line of |
60b2a8f
to
26adda0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @chengxiong-ruan and @postamar)
pkg/sql/catalog/descpb/index.go, line 73 at r2 (raw file):
explicitColIDs := desc.KeyColumnIDs[desc.ExplicitColumnStartIdx():] explicitColNames := desc.KeyColumnNames[desc.ExplicitColumnStartIdx():] colIDs := make([]ColumnID, 0, len(explicitColIDs))
nit: make this make(ColumnIDs, 0, len(explicitColIDs))
, return ColumnIDs
, and avoid the cast below?
pkg/sql/logictest/testdata/logic_test/hash_sharded_index, line 694 at r2 (raw file):
statement ok CREATE TABLE parent ( id INT PRIMARY KEY USING HASH WITH BUCKET_COUNT = 8
nit: https://sqlfum.pt/ or cockroach sqlfmt
your sql? It's jarring to see the newlines without the indentation.
Fixes cockroachdb#69192 Referencing table does not know about the shard column. So need to ignore it when looking for a valid unique constraint. Release note (bug fix): Foreign key referencing hash sharded key won't fail anymore.
26adda0
to
473e91c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @postamar)
pkg/sql/catalog/descpb/index.go, line 73 at r2 (raw file):
Previously, ajwerner wrote…
nit: make this
make(ColumnIDs, 0, len(explicitColIDs))
, returnColumnIDs
, and avoid the cast below?
👍 done
pkg/sql/logictest/testdata/logic_test/hash_sharded_index, line 694 at r2 (raw file):
Previously, ajwerner wrote…
nit: https://sqlfum.pt/ or
cockroach sqlfmt
your sql? It's jarring to see the newlines without the indentation.
TIL the sqlfmt
! nice~
TFTR! bors r+ |
Build succeeded: |
Fixes #69192
Referencing table does not know about the shard column. So
need to ignore it when looking for a valid unique constraint.
Release note (bug fix): Foreign key referencing hash sharded
key won't fail anymore.