sql,schemachanger: ADD PRIMARY KEY NOT VALID returns an error#97746
Conversation
|
This change is |
ajwerner
left a comment
There was a problem hiding this comment.
I'm glad this was a small change. I think the release note is hard to read. How about using examples? Maybe something like:
Release note (sql change): Prior to this change,
ALTER TABLE ... ADD PRIMARY KEY ... NOT VALIDwas supported, and resulted in theNOT VALIDclause being ignored. Now, theNOT VALIDclause will result in an error. This behavior matches that of postgres.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @Xiang-Gu)
pkg/sql/logictest/testdata/logic_test/alter_table line 2950 at r1 (raw file):
CREATE TABLE t_96729 (i INT NOT NULL) statement error PRIMARY KEY constraints cannot be marked NOT VALID
nit: add a pgcode to this test and make sure it matches postgres's
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/alter_table.go line 233 at r1 (raw file):
if d.PrimaryKey { if t.ValidationBehavior == tree.ValidationSkip { return errors.New("PRIMARY KEY constraints cannot be marked NOT VALID")
this needs a pgcode and so does the one in the declarative schema changer
2b5388e to
bdc8bfb
Compare
Xiang-Gu
left a comment
There was a problem hiding this comment.
Thanks for the prompt feedback. I've addressed them.
Reviewable status:
pkg/sql/alter_table.go line 233 at r1 (raw file):
Previously, ajwerner wrote…
this needs a pgcode and so does the one in the declarative schema changer
done
pkg/sql/logictest/testdata/logic_test/alter_table line 2950 at r1 (raw file):
Previously, ajwerner wrote…
nit: add a pgcode to this test and make sure it matches postgres's
done
ajwerner
left a comment
There was a problem hiding this comment.
Reviewed 1 of 4 files at r1, 5 of 6 files at r2, all commit messages.
Reviewable status:
|
TFTR! bors r+ |
|
bors r- |
|
Canceled. |
Release note (sql change): Previously, `ADD PRIMARY KEY NOT VALID` is processed by ignoring the NOT VALID qualifier. This is inadequate because PG throws an error instead. This PR ensures CRDB throws the same error, "PRIMARY KEY constraints cannot be marked NOT VALID", for such statements.
bdc8bfb to
5bc319b
Compare
|
bors r+ |
|
Build succeeded: |
Fixes #96729
Release note (sql change): When we add an unvalidated PK constraint to a table, assuming its PK was on the implicit "rowid" column, will return an error. This is compatible to what PG will do.