sql: add pg_catalog.pg_user view
#27161
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benesch
changed the title
pg_catalog.pg_user view
5 tasks
benesch
commented
May 18, 2024
Comment on lines
+5576
to
+5598
| -- We determine login status each time a role logs in, so there's no clean | ||
| -- way to accurately determine this in the catalog. Instead we do something | ||
| -- a little gross. For system roles, we hardcode the known roles that can | ||
| -- log in. For user roles, we determine `rolcanlogin` based on whether the | ||
| -- role name looks like an email address. | ||
| -- | ||
| -- This works for the vast majority of cases in production. Roles that users | ||
| -- log in to come from Frontegg and therefore *must* be valid email | ||
| -- addresses, while roles that are created via `CREATE ROLE` (e.g., | ||
| -- `admin`, `prod_app`) almost certainly are not named to look like email | ||
| -- addresses. | ||
| -- | ||
| -- For the moment, we're comfortable with the edge cases here. If we discover | ||
| -- that folks are regularly creating non-login roles with names that look | ||
| -- like an email address (e.g., `admins@sysops.foocorp`), we can change | ||
| -- course. | ||
| ( | ||
| r.name IN ('mz_support', 'mz_system') | ||
| -- This entire scheme is sloppy, so we intentionally use a simple | ||
| -- regex to match email addresses, rather than one that perfectly | ||
| -- matches the RFC on what constitutes a valid email address. | ||
| OR r.name ~ '^[^@]+@[^@]+\.[^@]+$' | ||
| ) AS rolcanlogin, |
There was a problem hiding this comment.
@MaterializeInc/adapter any qualms about this? Seemed like the best of the available options. We can always figure out how to do something more robust in the future.
There was a problem hiding this comment.
I don't love it, but I also don't have any better suggestions.
For Metaplane compatibility. To bring our `pg_user`, `pg_roles`, and `pg_authid` views in line with PostgreSQL, this commit also refactors how `rolcanlogin` is derived. We now do something a little gross but that will work well in practice to derive `rolcanlogin` from whether a role name looks like an email address, instead of hardcoding `NULL`. This makes the distinction between `pg_user` and `pg_roles` more useful: the former contains only the roles for which `rolcanlogin` is true, while the latter contains all roles. Split out from MaterializeInc#27155. Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
jkosh44
approved these changes
May 20, 2024
Comment on lines
+5576
to
+5598
| -- We determine login status each time a role logs in, so there's no clean | ||
| -- way to accurately determine this in the catalog. Instead we do something | ||
| -- a little gross. For system roles, we hardcode the known roles that can | ||
| -- log in. For user roles, we determine `rolcanlogin` based on whether the | ||
| -- role name looks like an email address. | ||
| -- | ||
| -- This works for the vast majority of cases in production. Roles that users | ||
| -- log in to come from Frontegg and therefore *must* be valid email | ||
| -- addresses, while roles that are created via `CREATE ROLE` (e.g., | ||
| -- `admin`, `prod_app`) almost certainly are not named to look like email | ||
| -- addresses. | ||
| -- | ||
| -- For the moment, we're comfortable with the edge cases here. If we discover | ||
| -- that folks are regularly creating non-login roles with names that look | ||
| -- like an email address (e.g., `admins@sysops.foocorp`), we can change | ||
| -- course. | ||
| ( | ||
| r.name IN ('mz_support', 'mz_system') | ||
| -- This entire scheme is sloppy, so we intentionally use a simple | ||
| -- regex to match email addresses, rather than one that perfectly | ||
| -- matches the RFC on what constitutes a valid email address. | ||
| OR r.name ~ '^[^@]+@[^@]+\.[^@]+$' | ||
| ) AS rolcanlogin, |
There was a problem hiding this comment.
I don't love it, but I also don't have any better suggestions.
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For Metaplane compatibility.
To bring our
pg_user,pg_roles, andpg_authidviews in line with PostgreSQL, this commit also refactors howrolcanloginis derived. We now do something a little gross but that will work well in practice to deriverolcanloginfrom whether a role name looks like an email address, instead of hardcodingNULL. This makes the distinction betweenpg_userandpg_rolesmore useful: the former contains only the roles for whichrolcanloginis true, while the latter contains all roles.Split out from #27155.
Motivation
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.pg_catalog.pg_userview for PostgreSQL compatibility.