Skip to content
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: add notice for Alter TYPE ADD VALUE IF NOT EXIST command #52534

Merged
merged 1 commit into from Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions pkg/sql/alter_type.go
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -74,6 +75,23 @@ func (n *alterTypeNode) startExec(params runParams) error {
func (p *planner) addEnumValue(
ctx context.Context, n *alterTypeNode, node *tree.AlterTypeAddValue,
) error {
if n.desc.Kind != descpb.TypeDescriptor_ENUM {
return pgerror.Newf(pgcode.WrongObjectType, "%q is not an enum", n.desc.Name)
}
// See if the value already exists in the enum or not.
for _, member := range n.desc.EnumMembers {
if member.LogicalRepresentation == node.NewVal {
if node.IfNotExists {
p.SendClientNotice(
ctx,
pgnotice.Newf("enum label %q already exists, skipping", node.NewVal),
)
return nil
}
return pgerror.Newf(pgcode.DuplicateObject, "enum label %q already exists", node.NewVal)
}
}

if err := n.desc.AddEnumValue(node); err != nil {
return err
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/notice
Expand Up @@ -33,3 +33,18 @@ ALTER TABLE d.t RENAME TO d.t2
----
NOTICE: renaming tables with a qualification is deprecated
HINT: use ALTER TABLE d.t RENAME TO t2 instead

statement ok
SET experimental_enable_enums=true;

# Start off with an empty enum, and add values to it.
statement ok
CREATE TYPE color AS ENUM ()

statement ok
ALTER TYPE color ADD VALUE 'black'

query T noticetrace
ALTER TYPE color ADD VALUE IF NOT EXISTS 'black'
----
NOTICE: enum label "black" already exists, skipping
20 changes: 2 additions & 18 deletions pkg/sql/sqlbase/type_desc.go
Expand Up @@ -254,25 +254,9 @@ func (desc *MutableTypeDescriptor) IsNew() bool {
}

// AddEnumValue adds an enum member to the type.
// AddEnumValue assumes that the type is an enum, and that the new value
// doesn't exist already in the enum.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nit -- don't also remove the comment that actually tells what the function does!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

func (desc *MutableTypeDescriptor) AddEnumValue(node *tree.AlterTypeAddValue) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the comment here now to say that "AddEnumValue assumes that the type is an enum, and that the new value doesn't exist already in the enum"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated comment, i will make sure to do such descriptive code comments in future contributions.

if desc.Kind != descpb.TypeDescriptor_ENUM {
return pgerror.Newf(pgcode.WrongObjectType, "%q is not an enum", desc.Name)
}
// See if the value already exists in the enum or not.
found := false
for _, member := range desc.EnumMembers {
if member.LogicalRepresentation == node.NewVal {
found = true
break
}
}
if found {
if node.IfNotExists {
return nil
}
return pgerror.Newf(pgcode.DuplicateObject, "enum label %q already exists", node.NewVal)
}

getPhysicalRep := func(idx int) []byte {
if idx < 0 || idx >= len(desc.EnumMembers) {
return nil
Expand Down