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: use qualifiable schema name for comment on schema #79055

Conversation

chengxiong-ruan
Copy link
Contributor

Previously we only use current db to resolve a schema when comment
on a schema. This is painful at least for our testing sometimes
because we need to switch db before commenting on a schema.

Release justification: low impact but can be useful for users.
Release note (sql change): COMMENT ON SCHEMA now can use qualified
schema name. So can do both COMMENT ON SCHEMA sc_name ... and
COMMENT ON SCHEMA db_name.sc_name ....

@chengxiong-ruan chengxiong-ruan requested a review from a team March 30, 2022 17:15
@chengxiong-ruan chengxiong-ruan requested a review from a team as a code owner March 30, 2022 17:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

:lgtm:

I don't think you need to our ought to backport this

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @chengxiong-ruan)


pkg/sql/comment_on_schema.go, line 47 at r1 (raw file):

		dbName = n.Name.Catalog()
	} else {
		// Users can't create a schema without being connected to a DB.

this comment is now stale, no?

Copy link
Contributor Author

@chengxiong-ruan chengxiong-ruan left a comment

Choose a reason for hiding this comment

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

yeah, I'll remove the flag before merging it.

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


pkg/sql/comment_on_schema.go, line 47 at r1 (raw file):

Previously, ajwerner wrote…

this comment is now stale, no?

ha yeah, my understanding on this comment was that we can always get a current db anyhow. but this seems not important anymore here, will remove it.

Previously we only use current db to resolve a schema when comment
on a schema. This is painful at least for our testing sometimes
because we need to switch db before commenting on a schema.

Release justification: low impact but can be useful for users.
Release note (sql change): `COMMENT ON SCHEMA` now can use qualified
schema name. So can do both `COMMENT ON SCHEMA sc_name ...` and
`COMMENT ON SCHEMA db_name.sc_name ...`.
@chengxiong-ruan chengxiong-ruan force-pushed the comment-on-schema-use-qulifiable-name branch from 4f259ec to 029239a Compare March 30, 2022 21:21
@chengxiong-ruan
Copy link
Contributor Author

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Mar 30, 2022

Build succeeded:

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