langium-sql icon indicating copy to clipboard operation
langium-sql copied to clipboard

Function call using SelectElements for parameters

Open cam-m opened this issue 1 year ago • 2 comments

I'm not sure the SimpleSelectStatement.select and PrimaryExpression.FunctionCall rules are correct.

I'm assuming FunctionCall is intended to parse system function calls (E.g. ABS() or DATEADD(), etc)

Problems:

  1. It currently uses the SelectElements rule for its params property;
{infer FunctionCall} function=GlobalReference '(' params=SelectElements? ')' 
      overClause=OverClause?

SelectElements:
    ('ALL'|distinct?='DISTINCT')? elements+=SelectElement (',' elements+=SelectElement)*
;
SelectElement infers SelectElement:
    {infer AllStar} '*'
    | {infer AllTable} variableName=[TableVariableSource:Identifier] '.' '*'
    | {infer ExpressionQuery} expr=Expression ('AS'? name=Identifier)?
;
  1. FunctionCall also directly defines the OVER statement, which should probably only be in SimpleSelectStatement in a window function expression

Happy to propose a fix if I've understood this correctly.

cam-m avatar Jan 25 '24 00:01 cam-m

  1. I think there are two kind of functions: Aggregators like SUM/MAX/MIN/COUNT where this rules make sense. And normal functions which I did not have in mind at the time of creating the grammar. Maybe it makes sense to define aggregators hardcoded and do it the right way for normal functions.
  2. The OVER clause has the same thinking behind it. Window functions are only used with aggregators AFAIK.

Side note: You can always allow more than possible and forbid what shall not be possible using a validator. This might help the user to understand differences in language concepts (with nice error messages presumed). But in this case, I think, hardcoded aggregator functions would be better.

Lotes avatar Jan 30 '24 07:01 Lotes

@Lotes Agreed, and i think aggregators only allow a narrow set of args - i think column name or alias.

I understand what you mean re the validator strategy. I see what i can put together.

cam-m avatar Jan 30 '24 22:01 cam-m