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: fix error message when renaming databases with dependent sequences #45427

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

otan
Copy link
Contributor

@otan otan commented Feb 26, 2020

Refs: #45411

Release note (bug fix): Previously, renaming a database with dependent
views returned a misleading error message that implies it was a
dependent view on a dependent table. This PR renames the error message
generically to cannot rename relation ... as it depends on relation ... instead.

@otan otan requested review from rohany and a team February 26, 2020 06:47
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rohany
Copy link
Contributor

rohany commented Feb 26, 2020

Changes LGTM, but I find the error message still a bit not informative enough. What about
cannot rename relation ... as it depends on relation ... in database ...?

@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

i've put the full schema lookup name on the depends on relation 'x' bit.

@rohany
Copy link
Contributor

rohany commented Feb 26, 2020

I think the table should have a qualified name as well then

@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

I think the table should have a qualified name as well then

ah yeah that's what i meant. PR update incoming, check the tests

@otan otan force-pushed the fix_error_message branch from 258190e to 5104de7 Compare February 26, 2020 19:33
@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

ah woops, missed a spot!

@otan otan force-pushed the fix_error_message branch from 5104de7 to fe4c3fb Compare February 26, 2020 19:47
@rohany
Copy link
Contributor

rohany commented Feb 26, 2020

The code LGTM, a good number of failing tests to fix though.

@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

hahaa just the one

Release note (bug fix): Previously, renaming a database with dependent
views returned a misleading error message that implies it was a
dependent view on a dependent table. This PR renames the error message
generically to `cannot rename relation ... as it depends on relation
...` instead.
@otan otan force-pushed the fix_error_message branch from fe4c3fb to 60a8ebe Compare February 26, 2020 20:40
@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

bors r=rohany
just a bench flake

@craig
Copy link
Contributor

craig bot commented Feb 26, 2020

Build succeeded

@craig craig bot merged commit 75f4ff3 into cockroachdb:master Feb 26, 2020
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.

3 participants