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

ESQL: Merge Verifier based function validation into the functions #116336

Open
5 tasks
Tracked by #106679
nik9000 opened this issue Nov 6, 2024 · 2 comments
Open
5 tasks
Tracked by #106679

ESQL: Merge Verifier based function validation into the functions #116336

nik9000 opened this issue Nov 6, 2024 · 2 comments
Labels
:Analytics/ES|QL AKA ESQL >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@nik9000
Copy link
Member

nik9000 commented Nov 6, 2024

Description

In ESQL we mostly validate functions by calling their resolveType methods but for functions that we borrowed from QL in the old, old days we added validation to Verifier so we didn't have to modify the QL classes. That was very hard when we were a branch. But now that we are moving on #106679 we can and should centralize this validation. We should also make sure that we use our normal unit testing for this.

These methods look like they are doing extra, out of band validation:

  • checkOperationsOnUnsignedLong
  • checkBinaryComparison
  • validateBinaryComparison
  • validateUnsignedLongOperator
  • validateUnsignedLongNegation
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Nov 6, 2024
@nik9000
Copy link
Member Author

nik9000 commented Nov 6, 2024

Not fixing this exposes us to fun like #116346. Not a terribly hard bug to fix, but it makes our unit tests very unreliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

No branches or pull requests

2 participants