search

trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.46k stars 3.01k forks source link

Names of CALL arguments do not follow the SQL Identifier semantics #11120

Open kasiafi opened 2 years ago

kasiafi commented 2 years ago

The argument name is taken textually, e.g. in CALL some_procedure(arg => 1) the argument name arg is not uppercased. Even worse, in CALL some_procedure("arg" => 1) , the argument name is not unquoted.

If we fix that to respect the proper semantics, it will likely break usages depending on the “textual” semantics.

I need to extend the named arguments for the sake of polymorphic table functions, and I don’t feel like spreading this error. For now, I'll add a parallel type to represent PTF named arguments with proper SQL Identifier semantics. Ideally, we should have single implementation consistent with the spec.

Please share any thoughts.

kasiafi commented 2 years ago

cc @martint @electrum @losipiuk the same applies to ALTER TABLE ... EXECUTE named arguments.

kasiafi commented 2 years ago

It's easy to fix: https://github.com/trinodb/trino/pull/11123

losipiuk commented 2 years ago

Why would we want to support mixed case argument names? It adds complexity to the codebase while (I think) not giving any benefit to the users.

kasiafi commented 2 years ago

Why would we want to support mixed case argument names? It adds complexity to the codebase while (I think) not giving any benefit to the users.

@losipiuk Mixed-case names are not the goal we want to achieve. Rather a side-effect of the fix. Also, I believe that it does not make the codebase considerably more complex. The actual fix is 3 lines.

We're trying to stick to the rules of the spec regarding identifiers. For some time now we've been cautious to handle identifiers correctly when adding new language features. And we plan to apply the correct semantics everywhere one day. The legacy semantics might seem more user-friendly, but consistent behavior is also important from the user's perspective.

kasiafi commented 2 years ago

@losipiuk I'm not insisting on changing argument semantics in TABLE EXECUTE. However, the polymorphic table functions use the same convention for arguments, and I think I will follow the proper semantics there. As a result, some confusion might arise from name => val behaving different in the context of TABLE EXECUTE than in the context of a PTF.