sql: `current` functions not consistent with PostgreSQL

morsapaes commented 1 year ago

What version of Materialize are you using?

v0.51.0

How did you install Materialize?

Materialize Cloud

What is the issue?

According to the PostgreSQL documentation:

current_catalog, current_role, current_schema, current_user, session_user, and user have special syntactic status in SQL: they must be called without trailing parentheses. In PostgreSQL, parentheses can optionally be used with current_schema, but not with the others.

We seem to break compatibility by allowing calls using trailing parentheses for current_schema, current_user and session_user (current_role behaves as expected).

materialize=> SELECT current_user;
     current_user
-----------------------
 marta@materialize.com
(1 row)

materialize=> SELECT current_user();
     current_user
-----------------------
 marta@materialize.com
(1 row)

materialize=> SELECT current_role;
     current_role
-----------------------
 marta@materialize.com
(1 row)

materialize=> SELECT current_role();
ERROR:  function current_role() does not exist
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

EDIT: @benesch proposes we lean into the more permissive take and allow calling all such functions with trailing parentheses in Materialize. See https://github.com/MaterializeInc/materialize/issues/18824#issuecomment-1745726526 for details.

However, after some digging, I found out that this is not a very trivial work (maybe I'm missing something). It seems like the functions current_schema, current_user and session_user are built-in functions that are parsed by the the role of function with parentheses. The reason why current_role works is because it is parsed as a special identifier that is transformed to a function call of current_user(): here.

Is there a way to reject user from calling these functions with parentheses?

OscarTHZhang commented 11 months ago

nvm, I found the place to make the prevention. PR is up: https://github.com/MaterializeInc/materialize/pull/20627. Need to also update docs accordingly.

benesch commented 9 months ago

From a discussion on Slack: I suggest we move in the opposite direction on this and:

Uniformly allow trailing parentheses on the current_*, session_user, and user functions.
Uniformly allow calling those functions without parentheses.

This will be a superset of PostgreSQL's behavior, so we'll remain compatible. I can't see a reason to match PostgreSQL's behavior of prohibiting trailing parentheses for everything but current_schema. It's just purely confusing for users.

Snowflake, FWIW, allows trailing parenthesis for all of these functions.

sdht0 commented 1 month ago

I've implemented the unification logic in #27171.

MaterializeInc / materialize