add a new table user_privileges

[yangliang9004]

sql: Complete the information_schema, add a new table user_privileges , please see #8675

[cockroach-teamcity]

This change is 

[benesch]

Thanks for your contribution, @yangliang9004! It's much appreciated. I'll defer the final decision to @jordanlewis (or someone else more familiar with the code), but I've left you some small style feedback in the meantime.

---

Reviewed 2 of 2 files at r1.
Review status: 1 of 2 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

pkg/sql/information_schema.go, line 422 at r1 (raw file):

var informationSchemaUserPrivileges = virtualSchemaTable{
	schema: `
CREATE  TABLE information_schema.user_privileges (

Stray space here between CREATE and TABLE.

---

pkg/sql/information_schema.go, line 424 at r1 (raw file):

CREATE  TABLE information_schema.user_privileges (
	GRANTEE STRING NOT NULL DEFAULT '',
	TABLE_CATALOG STRING NOT NULL  DEFAULT '',

Ditto here between NULL and DEFAULT.

---

pkg/sql/information_schema.go, line 432 at r1 (raw file):

			grantee       string
			privilegeType string
		}

FYI we avoid capitalized types in Go unless they're intended to be exported. But I think you can kill this entire type if you make the change suggested in the next comment.

---

pkg/sql/information_schema.go, line 439 at r1 (raw file):

		}
		for _, desc := range descs {
			privirages := desc.GetPrivileges().Show()

privileges, but you can also skip this variable entirely by making the method call in the for loop directly. See the next comment.

---

pkg/sql/information_schema.go, line 443 at r1 (raw file):

				for _, p := range u.Privileges {
					r := Drow{u.User, p}
					m[r] = struct{}{}

I don't think you actually use the de-duplicating properties of this map, so I think it'd be cleaner if you inlined the call to addRow. So:

for _, desc := range descs {
  for _, user := range desc.GetPrivileges().Show() {
    for _, p := range user.Privileges {
      if err := addRow(...); err != nil {
        return nil;
      }
    }
  } 
}

---

pkg/sql/testdata/logic_test/information_schema, line 925 at r1 (raw file):

tester1  def            INSERT          NULL         
tester2  def            SELECT          NULL         
tester2  def            UPDATE          NULL         

Looks like some trailing spaces slipped into these lines.

---

Comments from Reviewable

[yangliang9004]

@benesch , I have tried to do as you say, but the output as follows, it really have a lot of repetition, so I have to use the de-duplicating properties of this map, then it will have the correct output :

                        expected:
        		    grantee  table_catalog  privilege_type  is_grantable
        		    root     def            ALL             NULL
        		    root     def            DELETE          NULL
        		    root     def            GRANT           NULL
        		    root     def            INSERT          NULL
        		    root     def            SELECT          NULL
        		    root     def            UPDATE          NULL
        		but found (query options: "colnames") :
        		    grantee  table_catalog  privilege_type  is_grantable
        		    root     def            ALL             NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            DELETE          NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            GRANT           NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            INSERT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            SELECT          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL
        		    root     def            UPDATE          NULL

[yangliang9004]

@benesch I have deleted the redundant spaces in "pkg/sql/information_schema.go" and "pkg/sql/testdata/logic_test/information_schema"

[jordanlewis]

Thanks for your contribution!

This change is not quite correct, unfortunately. The user_privileges table encodes the global privileges for each user. This change instead encodes every privilege for each user, which is inaccurate. If you were hoping that this is the table to use to figure out what permissions are available to users, perhaps you meant to look at the schema_privileges or table_privileges tables in information_schema, which by the way we already support.

CockroachDB does not currently have the capability of modifying the global privileges for users. It only allows modifying the database-level or table-level privileges for users.

Therefore, this table should contain a single entry for every privilege against root - and that's it! You should not need to iterate over any descriptors.

Does that make sense? Can you make that change?

---

Review status: 0 of 2 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

Comments from Reviewable

[yangliang9004]

@jordanlewis Yeah ! I can make this change .

[yangliang9004]

@jordanlewis HI, I have made the change , look forward to your advice. Thanks.

[jordanlewis]

Excellent! This looks correct. I have one small comment, after you address it we'll be good to merge.

---

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.

---

pkg/sql/information_schema.go, line 432 at r2 (raw file):

		for _, p := range privilege.List(privilege.ByValue[:]).SortedNames() {
			if err := addRow(
				parser.NewDString(security.RootUser), // grantee

Pull this out of the loop - we don't need to reallocate this every time.

---

Comments from Reviewable

[yangliang9004]

@jordanlewis do you mean that I shoud write as the follows:

pl := privilege.List(privilege.ByValue[:]).SortedNames()
for _, p := range pl {
			if err := addRow(
				parser.NewDString(security.RootUser),

[jordanlewis]

No, I mean:

grantee := parser.NewDString(security.RootUser)
for _, p := range privilege.List(privilege.ByValue[:]).SortedNames() {
  if err := addRow(
    grantee, // grantee
...

[yangliang9004]

you are right , I get it .

[yangliang9004]

@jordanlewis I have change as the comment say, look forward to your advice. Thanks.

[jordanlewis]

---

Review status: 1 of 2 files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.

---

Comments from Reviewable

[jordanlewis]

Thanks @yangliang9004!