From 3a3123d97b2edbdc1b17d497c0087bb728b3b358 Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Fri, 4 Nov 2022 15:02:48 -0400 Subject: [PATCH] scbuild: error on duplicate columns in ALTER PRIMARY KEY This check has been missing from the declarative schema changer builder. This would result in execution errors in later stages when the new index makes it into the table descriptor. Since these are internal errors, the user experience is correspondingly poor. This patch fixes this. Fixes #91301 Release note (bug fix): fixed a bug present only in earlier 22.2 release candidates, in which an ALTER PRIMARY KEY USING COLUMNS (x, x) statement would result in an internal error instead of the expected user-facing error with a pg-code. --- pkg/sql/logictest/testdata/logic_test/alter_primary_key | 3 +++ .../internal/scbuildstmt/alter_table_alter_primary_key.go | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/pkg/sql/logictest/testdata/logic_test/alter_primary_key b/pkg/sql/logictest/testdata/logic_test/alter_primary_key index 06512541bcaf..2d96b9c2ffd9 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_primary_key +++ b/pkg/sql/logictest/testdata/logic_test/alter_primary_key @@ -4,6 +4,9 @@ CREATE TABLE t (x INT PRIMARY KEY, y INT NOT NULL, z INT NOT NULL, w INT, INDEX statement ok INSERT INTO t VALUES (1, 2, 3, 4), (5, 6, 7, 8) +statement error pgcode 0A000 .* contains duplicate column \"y\" +ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y, y) + statement ok ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y, z) diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go index 113e42338fdd..96b6575809fd 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go @@ -208,6 +208,7 @@ func checkForEarlyExit(b BuildCtx, tbl *scpb.Table, t alterPrimaryKeySpec) { panic(err) } + usedColumns := make(map[tree.Name]bool, len(t.Columns)) for _, col := range t.Columns { if col.Column == "" && col.Expr != nil { panic(errors.WithHint( @@ -219,6 +220,11 @@ func checkForEarlyExit(b BuildCtx, tbl *scpb.Table, t alterPrimaryKeySpec) { "use columns instead", )) } + if usedColumns[col.Column] { + panic(pgerror.Newf(pgcode.FeatureNotSupported, + "new primary key contains duplicate column %q", col.Column)) + } + usedColumns[col.Column] = true colElems := b.ResolveColumn(tbl.TableID, col.Column, ResolveParams{ IsExistenceOptional: false,