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

[ES|QL] FunctionCallExpression, UnaryExpression, and BinaryExpression differentiation #189259

Closed
vadimkibana opened this issue Jul 26, 2024 · 2 comments · Fixed by #189268
Closed
Assignees
Labels
Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana

Comments

@vadimkibana
Copy link
Contributor

vadimkibana commented Jul 26, 2024

Currently in ES|QL AST all function call expressions are represented by a node of type function:

{type: 'function', name: 'fn', args: []}

However, binary and unary expressions are also represented by that same node type, for example ... AND ... binary logical expression:

{type: 'function', name: 'and', args: []}

This is a problem for at least two scenarios:

  1. When pretty printing, there is not enough information to know if the node should be printed as a function call or as a binary expression. For example, how do you print the {type: 'function', name: 'and'} node? Like binary expression?
y AND y

Or like a function call?

AND(x, y)
  1. Users will be able to define custom functions going forward. The custom function names may collide with the expression names we use. For example, for a regex expression like ... NOT RLIKE ... we create a node like {type: 'function', name: 'not_rlike'}. This means it will collide with a custom function if ES|QL grammar or a user will define a function called not_rlike() at some point.

Solution: introduce subtype field for function AST nodes. Like

{type: 'function', subtype: 'variadic-call', name: 'fn'} // fn(1, 2, 3)
{type: 'function', subtype: 'binary-expression', name: 'and'} // x AND y
@botelastic botelastic bot added the needs-team Issues missing a team label label Jul 26, 2024
@vadimkibana vadimkibana added Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana labels Jul 26, 2024
@vadimkibana vadimkibana self-assigned this Jul 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jul 26, 2024
@drewdaemon
Copy link
Contributor

My 2 🪙:

The key here is to make sure we stay compatible with the function definitions provided by Elasticsearch. I can see value in this idea, but definitely not worth it if it comes at the expense of our function definition sync automation (or if it significantly complicates it).

Users will be able to define custom functions going forward.

It is not clear when or how. I imagine that whenever this happens, there will be conflict checks at the Elasticsearch level. So, I don't see this as something we really need to worry about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants