sql: fix username parsing for CURRENT_USER/SESSION_USER

[RichardJCai]

Release note (sql change): Fix bug where previously CURRENT_USER and
SESSION_USER were parsed incorrectly.

Fixes #54696

[cockroach-teamcity]

This change is 

[knz]

I reviewed the tree and parser changes only. Generally OK, but I think there's some unnecessary complexity related to the formatting code.

Reviewed 23 of 38 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @rafiss, and @RichardJCai)

---

pkg/sql/sem/tree/alter_database.go, line 16 at r1 (raw file):

type AlterDatabaseOwner struct {
	Name Name
	// TODO(solon): Adjust this, see

Here and elsewhere, remove the TODOs that refer to this specific issue.

---

pkg/sql/sem/tree/format.go, line 389 at r1 (raw file):

// FormatRoleSpec formats a role specification safely.
func (ctx *FmtCtx) FormatRoleSpec(spec RoleSpec) {

I see you also have a Format method on type RoleSpec
Then you don't need this one. Instead, do ctx.FormatNode(&n.RoleSpec) on the other nodes, which will also be easier to read and less error-prone.

[dt]

how disruptive to your work would it be to wait on merging sweeping refactors like this, at least if they're also hitting bulk io's packages, until we have a 21.2 RC cut? some teams, mine included, are trying to minimize refactor churn until RC 1 is tagged, so that if we need to prepare an urgent fix for a release-blocking issue, we don't have as much risk of unclean backports delaying it further

[RichardJCai]

how disruptive to your work would it be to wait on merging sweeping refactors like this, at least if they're also hitting bulk io's packages, until we have a 21.2 RC cut? some teams, mine included, are trying to minimize refactor churn until RC 1 is tagged, so that if we need to prepare an urgent fix for a release-blocking issue, we don't have as much risk of unclean backports delaying it further

This can definitely wait, I'm actually just picking this up and other smaller issues because I'm waiting on the 21.2 RC cut to merge my other huge PR.

[rafiss]

this is great! just had a nit, then feel free to merge after 21.2 RC

after this are there any other usages of name_list or name for usernames in sql.y?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @knz, and @RichardJCai)

---

pkg/sql/sem/tree/create.go, line 1577 at r2 (raw file):

	// TODO(solon): Adjust this, see
	// https://github.com/cockroachdb/cockroach/issues/54696
	AuthRole RoleSpec

nit: remove the todo

[RichardJCai]

This should be ok to merge now right?

[knz]

This should be ok to merge now right?

I don't have further comments, but there's still one unaddressed comment by rafi.

Reviewed 1 of 38 files at r1, 8 of 11 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @knz, and @RichardJCai)

[RichardJCai]

This should be ok to merge now right?

I don't have further comments, but there's still one unaddressed comment by rafi.

Reviewed 1 of 38 files at r1, 8 of 11 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @knz, and @RichardJCai)

Ah yeah I haven't made those changes yet, more asking in reference to about waiting for 21.2 RC before merging this,

[dt]

asking in reference to about waiting for 21.2 RC before merging this

I no longer have any concern about refactor churn, at least w.r.t. bulk's packages, so no objection here.

[RichardJCai]

bors r=rafiss

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[RichardJCai]

Fixed merge skew
bors r=rafiss

[craig]

Build succeeded:

• GitHub CI (Cockroach)