sql: OWNED BY SESSION_USER/CURRENT_USER does not work

[arulajmani]

CURRENT_USER/SESSION_USER should be special keywords, but our current syntax treats them as normal strings. Consider the following example using the ALTER SCHEMA command:

root@127.0.0.1:61524/movr> alter schema sc OWNER TO CURRENT_USER;
ERROR: role/user "current_user" does not exist
SQLSTATE: 42704
root@127.0.0.1:61524/movr> alter schema sc OWNER TO SESSION_USER;
ERROR: role/user "session_user" does not exist
SQLSTATE: 42704
root@127.0.0.1:61524/movr> alter schema sc OWNER TO admin;
ALTER SCHEMA

Time: 0.029s total (exec 10.2% / net 88.2% / other 1.6%)

Epic CRDB-8948

[angelapwen]

Curious about this one — the syntax does parse it with role_spec which takes in any non-reserved string, CURRENT_USER, or SESSION_USER (and is also used in many other situations where this is the case).

[angelapwen]

@knz you mentioned in #55382 that DROP ROLE handles this case appropriately. This is my output when I tested:

demo@127.0.0.1:52133/movr> DROP ROLE SESSION_USER;
invalid syntax: statement ignored: at or near "session_user": syntax error
SQLSTATE: 42601
DETAIL: source SQL:
DROP ROLE SESSION_USER
                      ^
HINT: try \h DROP ROLE

I don't know that this is the right error either — did you get this output?

[angelapwen]

Referencing the way Postgres handles this situation: RoleSpec is a new parse tree node created:

https://github.com/postgres/postgres/blob/45b9805706fdc726906fc9187c9a7b38c559755c/src/include/nodes/parsenodes.h#L315-L333

https://github.com/postgres/postgres/blob/e578c17d81662b4f4f63a2797bc1be64af3c8f93/src/backend/parser/gram.y#L14976-L14987

https://github.com/postgres/postgres/blob/e578c17d81662b4f4f63a2797bc1be64af3c8f93/src/backend/parser/gram.y#L16181-L16193

It seems we may want to use the same format to work on this bug.

[knz]

I meant that DROP ROLE doesn't use simple strings for its usernames. Instead it allows names to be expressions, in particular so that the SQL client can use placeholders.

The key piece of parser insight is this:

%type <tree.Expr> string_or_placeholder_list
...
string_or_placeholder:
  non_reserved_word_or_sconst
  {
    $$.val = tree.NewStrVal($1)
  }
| PLACEHOLDER
  {
    p := $1.placeholder()
    sqllex.(*lexer).UpdateNumPlaceholders(p)
    $$.val = p
  }

string_or_placeholder_list:
  string_or_placeholder
  {
    $$.val = tree.Exprs{$1.expr()}
  }
| string_or_placeholder_list ',' string_or_placeholder
  {
    $$.val = append($1.exprs(), $3.expr())
  }
drop_role_stmt:
  DROP ROLE string_or_placeholder_list
  {
    $$.val = &tree.DropRole{Names: $3.exprs(), IfExists: false, IsRole: $2.bool()}
  }

And the key piece of useful Go code is this:

// DropRoleNode creates a "drop user" plan node. This can be called from DROP USER or DROP ROLE.
func (p *planner) DropRoleNode(...) {
       ...
       names, err := p.TypeAsStringArray(ctx, namesE, opName)
       ...
}

func (n *DropRoleNode) startExec(params runParams) error {
       ...
        names, err := n.names()
}

I think all our role handling throughout SQL should ensure that names are handled this way.

[knz]

This code does not Currently handle session_user / current_user but the way to handle this is to make the string_or_placeholder rule instantiate a FuncExpr for it.

[angelapwen]

I'm tying up loose ends as I offboard this week and want to make sure that this issue is being tracked in the new SQL Experience board. I've removed myself from assigned so others will feel free to take it on. Thank you ✨

[rafiss]

I found that Postgres does not use a function for SESSION_USER or CURRENT_USER. The parser instead looks for these tokens and makes a "special" RoleSpec: https://github.com/postgres/postgres/blob/6beb38cfc9ddd4cd3d2eb5402981ebdd69a618b4/src/backend/parser/gram.y#L15315-L15326

When the RoleSpec is accessed, these types are special-cased: https://github.com/postgres/postgres/blob/6beb38cfc9ddd4cd3d2eb5402981ebdd69a618b4/src/backend/utils/adt/acl.c#L5128-L5146