Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Add option to specify custom argument description/list #860

Open
Yury-Fridlyand opened this issue Sep 28, 2022 · 2 comments
Open
Labels
enhancement New feature or request

Comments

@Yury-Fridlyand
Copy link
Collaborator

Is your feature request related to a problem?

A error returned to user in case if argument(s) are missing or have wrong type(s) is not user-friendly and usually confusing. As more signatures/arguments function has as less readable error become. See examples:

opensearchsql> SELECT PI(1);
{'reason': 'Invalid SQL query', 'details': 'pi function expected {[]}, but get [INTEGER]', 'type': 'ExpressionEvaluationException'}
opensearchsql> SELECT DATEDIFF();
{'reason': 'Invalid SQL query', 'details': 'datediff function expected {[DATE,DATE],[DATETIME,DATE],[DATE,DATETIME],[DATETIME,DATETIME],[DATE,TIME],[TIME,DATE],[TIME,TIME],[TIMESTAMP,DATE],[DATE,TIMESTAMP],[TIMESTAMP,TIMESTAMP],[TIMESTAMP,TIME],[TIME,TIMESTAMP],[TIMESTAMP,DATETIME],[DATETIME,TIMESTAMP],[TIME,DATETIME],[DATETIME,TIME]}, but get []', 'type': 'ExpressionEvaluationException'}

The goal is also to avoid such ugly messages like these:

assertEquals("simple_query_string function expected {[STRUCT,STRING],[STRUCT,STRING,STRING],"
+ "[STRUCT,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING],[STRUCT,STRING,"
+ "STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING,STRING,STRING],"
+ "[STRUCT,STRING,STRING,STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING],[STRUCT,STRING,STRING,"
+ "STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING]},"
+ " but get [STRUCT]",

What solution would you like?

Add new field to FunctionResolver to store argument/function description. It should be returned (if given) instead of default message, see

private String formatFunctions(Set<FunctionSignature> functionSignatures) {
return functionSignatures.stream().map(FunctionSignature::formatTypes)
.collect(Collectors.joining(",", "{", "}"));
}

What alternatives have you considered?

N/a

Do you have any additional context?

@Yury-Fridlyand Yury-Fridlyand added enhancement New feature or request untriaged labels Sep 28, 2022
@MaxKsyunz
Copy link
Collaborator

Using SELECT PI(1) and SELECT DATEDIFF() as examples, what error message would you like to see?

A common pattern is to say that requested signature is not supported.

SELECT PI(1) it could be PI([INTEGER]) is not a valid function call and similarly for SELECT DATEDIFF() -- DATEDIFF() is not a valid function call.

@Yury-Fridlyand
Copy link
Collaborator Author

For example, for DATEDIFF - DATEDIFF function gets two arguments of any datetime type, but for TIMEDIFF - TIMEDIFF function gets two arguments of any, but same datetime type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants