Skip to content

sql/schemachanger: detect unsupported types for unique indexes#84186

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
fqazi:addColumnRollback
Jul 12, 2022
Merged

sql/schemachanger: detect unsupported types for unique indexes#84186
craig[bot] merged 1 commit intocockroachdb:masterfrom
fqazi:addColumnRollback

Conversation

@fqazi
Copy link
Copy Markdown
Collaborator

@fqazi fqazi commented Jul 11, 2022

Fixes: #84185

Previously, the declarative schema changer when adding
a column with a unique index did not generate the
appropriate error when an unsupported type was encountered.
As a result, we would wait till execution and require a
rollback during execution phase. To address this, this patch
will report the unsupported type directly within the builder.

Release note: None

@fqazi fqazi requested a review from a team July 11, 2022 16:57
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown

@postamar postamar left a comment

Choose a reason for hiding this comment

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

LGTM, just nits.

msg := "column %s is of type %s and thus is not indexable"
typInfo := spec.colType.Type.DebugString()
msg = fmt.Sprintf(msg, d.Name, spec.colType.Type.Name())
panic(unimplemented.NewWithIssueDetailf(35730, typInfo, "%s", msg))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: if possible, replace this with

panic(unimplemented.NewWithIssueDetailf(35730, typInfo, msg, d.Name, spec.colType.Type.Name())

so that the error message redactor doesn't redact the whole message into meaninglessness.

subtest add_column_non_indexable_type

statement ok
CREATE TABLE t1_non_indexable(n int);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: pipe this statement and the one below through sqlfum.pt for nicer formatting

Fixes: cockroachdb#84185

Previously, the declarative schema changer when adding
a column with a unique index did not generate the
appropriate error when an unsupported type was encountered.
As a result, we would wait till execution and require a
rollback during execution phase. To address this, this patch
will report the unsupported type directly within the builder.

Release note: None
@fqazi fqazi force-pushed the addColumnRollback branch from b06db96 to dc744ca Compare July 11, 2022 20:16
Copy link
Copy Markdown
Collaborator Author

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

@postamar TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @postamar)

@fqazi
Copy link
Copy Markdown
Collaborator Author

fqazi commented Jul 12, 2022

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 12, 2022

Build succeeded:

@craig craig bot merged commit 571bfa3 into cockroachdb:master Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql/schemachanger: detect unsupported types for unique indexes

3 participants