langium-sql
langium-sql copied to clipboard
Function call using SelectElements for parameters
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:
- 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)?
;
- 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.
- 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.
- 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 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.