Skip to content

sql: simplify the role option formatting logic#72567

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20211109-simplify-pw
Nov 9, 2021
Merged

sql: simplify the role option formatting logic#72567
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20211109-simplify-pw

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Nov 9, 2021

Release note: None

@knz knz requested review from RichardJCai and rafiss November 9, 2021 12:43
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@knz knz force-pushed the 20211109-simplify-pw branch from c1fd57d to bbda7e3 Compare November 9, 2021 17:11
Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)


pkg/sql/roleoption/role_option.go, line 97 at r1 (raw file):

	"LOGIN":                  LOGIN,
	"NOLOGIN":                NOLOGIN,
	"VALID UNTIL":            VALIDUNTIL,

@RichardJCai could you confirm that this part of the change is backwards-compatible? i.e., is this string used as a value in system.role_options ?

@RichardJCai
Copy link
Copy Markdown
Contributor

RichardJCai commented Nov 9, 2021

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)

pkg/sql/roleoption/role_option.go, line 97 at r1 (raw file):

	"LOGIN":                  LOGIN,
	"NOLOGIN":                NOLOGIN,
	"VALID UNTIL":            VALIDUNTIL,

@RichardJCai could you confirm that this part of the change is backwards-compatible? i.e., is this string used as a value in system.role_options ?

IIUC this is only for formatting and won't be stored in system.role_options.

The part that's written to disk is unchanged here and lives in this map

var toSQLStmts = map[Option]string{
	CREATEROLE:             `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CREATEROLE')`,
	NOCREATEROLE:           `DELETE FROM system.role_options WHERE username = $1 AND option = 'CREATEROLE'`,
	LOGIN:                  `DELETE FROM system.role_options WHERE username = $1 AND option = 'NOLOGIN'`,
	NOLOGIN:                `UPSERT INTO system.role_options (username, option) VALUES ($1, 'NOLOGIN')`,
	VALIDUNTIL:             `UPSERT INTO system.role_options (username, option, value) VALUES ($1, 'VALID UNTIL', $2::timestamptz::string)`,
	CONTROLJOB:             `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CONTROLJOB')`,
	NOCONTROLJOB:           `DELETE FROM system.role_options WHERE username = $1 AND option = 'CONTROLJOB'`,
	CONTROLCHANGEFEED:      `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CONTROLCHANGEFEED')`,
	NOCONTROLCHANGEFEED:    `DELETE FROM system.role_options WHERE username = $1 AND option = 'CONTROLCHANGEFEED'`,
	CREATEDB:               `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CREATEDB')`,
	NOCREATEDB:             `DELETE FROM system.role_options WHERE username = $1 AND option = 'CREATEDB'`,
	CREATELOGIN:            `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CREATELOGIN')`,
	NOCREATELOGIN:          `DELETE FROM system.role_options WHERE username = $1 AND option = 'CREATELOGIN'`,
	VIEWACTIVITY:           `UPSERT INTO system.role_options (username, option) VALUES ($1, 'VIEWACTIVITY')`,
	NOVIEWACTIVITY:         `DELETE FROM system.role_options WHERE username = $1 AND option = 'VIEWACTIVITY'`,
	CANCELQUERY:            `UPSERT INTO system.role_options (username, option) VALUES ($1, 'CANCELQUERY')`,
	NOCANCELQUERY:          `DELETE FROM system.role_options WHERE username = $1 AND option = 'CANCELQUERY'`,
	MODIFYCLUSTERSETTING:   `UPSERT INTO system.role_options (username, option) VALUES ($1, 'MODIFYCLUSTERSETTING')`,
	NOMODIFYCLUSTERSETTING: `DELETE FROM system.role_options WHERE username = $1 AND option = 'MODIFYCLUSTERSETTING'`,
}

Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

thanks for confirming, and thanks for this change

@knz knz force-pushed the 20211109-simplify-pw branch from bbda7e3 to d861a35 Compare November 9, 2021 18:15
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Nov 9, 2021

TFYRs!

bors r=rafiss,RichardJCai

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 9, 2021

Build succeeded:

@craig craig bot merged commit 8b3d6fc into cockroachdb:master Nov 9, 2021
@knz knz deleted the 20211109-simplify-pw branch November 10, 2021 10:14
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.

4 participants