sql: grant to all tables did not skip privileges on sequences

[fqazi]

Previously, grant privilege to all tables attempted to apply privileges meant only for tables on to sequences. This could lead to validation error preventing certain combinations like "GRANT BACKUP ON ALL TABLES..." from working correctly. To address this, this patch adds support for correctly propogating the object type and skipping over unsupported privileges. When an unsupported privilege is encountered on an object other then a sequence a warning is now logged.

Fixes: #117861

Release note (bug fix): GRANT <...> ON ALL TABLES could fail if sequences existed and they did not support a privilege (for example BACKUP).

[cockroach-teamcity]

This change is 

[rafiss]

just had a nit to clean things up a bit, nice fix!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)

---

pkg/sql/grant_revoke.go line 288 at r1 (raw file):

				// Ensure we are only setting privilleges valid for this object type.
				// i.e. We only expect this for sequences.
				validPrivs, err := privilege.GetValidPrivilegesForObject(objType)

nit: all the new logic could be done once outside of the for loop

---

pkg/sql/grant_revoke.go line 302 at r1 (raw file):

					}
					if objType != privilege.Sequence {
						log.Warningf(ctx, "object type %s does not support privilege %v", objType, missingPrivs.SortedDisplayNames())

instead of a warning log, could we send this with BufferClientNotice? then i think it would also make sense to remove the objType != privilege.Sequence check

[fqazi]

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

---

pkg/sql/grant_revoke.go line 302 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

instead of a warning log, could we send this with BufferClientNotice? then i think it would also make sense to remove the objType != privilege.Sequence check

Good point and I generalized the logic from the original SEQUENCE logic.

[rafiss]

lgtm!

Reviewable status: complete! 0 of 0 LGTMs obtained

[fqazi]

@rafiss TFTR!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)
• check_generated_code
• docker_image_amd64
• examples_orms
• macos_amd64_build
• macos_arm64_build
• windows_build

[fqazi]

blathers backport 23.2

[fqazi]

blathers backport 23.1

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 007005f to blathers/backport-release-23.1-120685: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1 failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}