Skip to content

[ES|QL] Fixes the wrong validation when a named param is used as function#213355

Merged
stratoula merged 1 commit intoelastic:mainfrom
stratoula:fix-wrong-validation-stats
Mar 6, 2025
Merged

[ES|QL] Fixes the wrong validation when a named param is used as function#213355
stratoula merged 1 commit intoelastic:mainfrom
stratoula:fix-wrong-validation-stats

Conversation

@stratoula
Copy link
Contributor

@stratoula stratoula commented Mar 6, 2025

Summary

Fixes the bug described here #192255 (comment)

image

Checklist

@stratoula stratoula changed the title [ES|QL] Fixes the wrong validation when a named param is used as functions Mar 6, 2025
@stratoula stratoula mentioned this pull request Mar 6, 2025
13 tasks
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #49 / Alerting alerts_as_data alerts as data should not recover alert if the activeCount did not reach the alertDelay threshold with AAD

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.6MB 3.6MB +10.0B
@stratoula stratoula added release_note:fix v9.1.0 v8.19.0 Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// backport:version Backport to applied version labels labels Mar 6, 2025
@stratoula stratoula marked this pull request as ready for review March 6, 2025 14:07
@stratoula stratoula requested a review from a team as a code owner March 6, 2025 14:07
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// * or if it's a operators function, then all operands are agg functions or literals
// * or if it's a eval function then all arguments are agg functions or literals
// * or if a named param is used
function checkFunctionContent(arg: ESQLFunction) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: What is the motivation for defining checkFunctionContent and other functions inside statsValidator? Shall we move outside to the top level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this makes sense, will taje a look tmr in a follow up PR

@stratoula stratoula merged commit 4a8af4c into elastic:main Mar 6, 2025
22 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/13701207530

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 6, 2025
…tion (elastic#213355)

## Summary

Fixes the bug described here
elastic#192255 (comment)

<img width="1094" alt="image"
src="https://github.com/user-attachments/assets/69d4f004-6a66-416b-8aa6-e477b0380010"
/>

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 4a8af4c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Mar 6, 2025
…s function (#213355) (#213407)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Fixes the wrong validation when a named param is used as
function (#213355)](#213355)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2025-03-06T14:39:44Z","message":"[ES|QL]
Fixes the wrong validation when a named param is used as function
(#213355)\n\n## Summary\n\nFixes the bug described
here\nhttps://github.com//issues/192255#issuecomment-2684125449\n\n\n<img
width=\"1094\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/69d4f004-6a66-416b-8aa6-e477b0380010\"\n/>\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"4a8af4ca27f01e6f7d992e61c1f59feeff347a29","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:ES|QL","Team:ESQL","backport:version","v9.1.0","v8.19.0"],"title":"[ES|QL]
Fixes the wrong validation when a named param is used as
function","number":213355,"url":"https://github.com/elastic/kibana/pull/213355","mergeCommit":{"message":"[ES|QL]
Fixes the wrong validation when a named param is used as function
(#213355)\n\n## Summary\n\nFixes the bug described
here\nhttps://github.com//issues/192255#issuecomment-2684125449\n\n\n<img
width=\"1094\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/69d4f004-6a66-416b-8aa6-e477b0380010\"\n/>\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"4a8af4ca27f01e6f7d992e61c1f59feeff347a29"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/213355","number":213355,"mergeCommit":{"message":"[ES|QL]
Fixes the wrong validation when a named param is used as function
(#213355)\n\n## Summary\n\nFixes the bug described
here\nhttps://github.com//issues/192255#issuecomment-2684125449\n\n\n<img
width=\"1094\"
alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/69d4f004-6a66-416b-8aa6-e477b0380010\"\n/>\n\n###
Checklist\n\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"4a8af4ca27f01e6f7d992e61c1f59feeff347a29"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
…tion (elastic#213355)

## Summary

Fixes the bug described here
elastic#192255 (comment)


<img width="1094" alt="image"
src="https://github.com/user-attachments/assets/69d4f004-6a66-416b-8aa6-e477b0380010"
/>

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:ES|QL ES|QL related features in Kibana release_note:fix Team:ESQL ES|QL related features in Kibana t// v8.19.0 v9.1.0

4 participants