Bug: This upfront guard exclusively requires VIEWSCHEDULE on GlobalPrivilegeObject (a synthetic privilege object), so the VIEWSYSTEMTABLE fallback in authorization.go:264-273 — which only applies to system table descriptors — is never reached. Any non-admin user who currently accesses SHOW SCHEDULES via VIEWSYSTEMTABLE or direct SELECT grants on system.scheduled_jobs/system.jobs will get "does not have VIEWSCHEDULE system privilege" after this change, even though they have sufficient access to the underlying data.

The simplest fix is to remove this upfront guard entirely: the delegated query (SELECT ... FROM system.scheduled_jobs) will naturally hit the table-level privilege checks in authorization.go, which already correctly handle VIEWSYSTEMTABLE, VIEWSCHEDULE, and direct SELECT grants. Alternatively, expand this check to also accept VIEWSYSTEMTABLE.

Note that the analogous SHOW JOBS command has no such upfront gate, confirming this is inconsistent.

I agree with this comment. do we actually need the check here if there already is one in authorization.go?

We should be able to confirm this via a logictest.

Good catch on the VIEWSYSTEMTABLE regression. I tested removing the upfront guard entirely and the behavior is:

• Without the guard, the error is "user testuser does not have SELECT privilege on relation scheduled_jobs" — which doesn't tell the user what privilege they need.
• With the guard, we get a clear "does not have VIEWSCHEDULE system privilege".

So the guard serves a UX purpose (clear error message), but as written it blocks VIEWSYSTEMTABLE users which is a regression. I've updated the guard to also check VIEWSYSTEMTABLE — if the user has neither VIEWSCHEDULE nor VIEWSYSTEMTABLE, they get the clear error. If they have VIEWSYSTEMTABLE, the guard passes and the query executes with the implicit SELECT from authorization.go.

I've also added a logictest case confirming that VIEWSYSTEMTABLE still works for SHOW SCHEDULES.

I agree with this comment. do we actually need the check here if there already is one in authorization.go?

We should be able to confirm this via a logictest.

maybe worth refactoring this out into a function

Have used the RequireViewEventLogPermission function here which checks both VIEWEVENTLOG and VIEWCLUSTERMETADATA internally

Bug: Both CheckPrivilege calls use == nil to test the result, which swallows all errors — not just InsufficientPrivilege. If CheckPrivilege fails due to a transient error (KV unavailability, context cancellation, descriptor resolution failure, etc.), the code will misinterpret it as "user lacks the privilege" and return a misleading InsufficientPrivilege error to an authorized user.

The established pattern in this package (see show_all_cluster_settings.go:28-34, show_default_session_variables_for_role.go:31-35) correctly distinguishes permission errors from operational errors. Consider:

i think this is valid

Done — addressed in dc4e9c5. Also replaced VIEWSCHEDULE with VIEWJOB (per updated requirements) and properly distinguished InsufficientPrivilege from transient errors, matching the pattern in show_all_cluster_settings.go and show_default_session_variables_for_role.go.

nit: the use of "it" here is confusing
maybe something like error if the user does not have any or error otherwise

Done — changed to error if the user does not have any in dc4e9c5.

i think this is valid

this error branching makes me a little nervous-- i had to convince myself that if err != nil, then we always return an error.

could you make this cleaner by calling a helper on the error path? i.e.

if err := cat.CheckPrivilege(d.ctx, globalPrivObj, user, privilege.VIEWJOB); err != nil {
   return processPrivError(err)
}

Done — extracted a processPrivError helper that returns nil for InsufficientPrivilege and propagates unexpected errors. The flow is now: check VIEWJOB → if missing, check VIEWSYSTEMTABLE → if both missing, return privilege error.