Skip to content

[8.x] [ES|QL] Deep check if CHANGE_POINT ON option has any contents (#216093)#216136

Merged
kibanamachine merged 2 commits intoelastic:8.xfrom
kibanamachine:backport/8.x/pr-216093
Mar 31, 2025
Merged

[8.x] [ES|QL] Deep check if CHANGE_POINT ON option has any contents (#216093)#216136
kibanamachine merged 2 commits intoelastic:8.xfrom
kibanamachine:backport/8.x/pr-216093

Conversation

@kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

…astic#216093)

## Summary

When `ON` option in `CHANGE_POINT` has no value, it is invalid syntax
according ANTLR grammar. However, ANTLR does not fail but succeeds and
returns a column name, which is an empty string.

Hence, in the past we created an `ON` option with an empty column name,
but marked as `incomplete: true`, beucause ANTLR at least told us that.

<img width="295" alt="Screenshot 2025-03-26 at 18 13 32"
src="https://github.com/user-attachments/assets/b5fd4512-04a2-48b3-b874-1ea35a3c7e84"
/>

All of the above, IMO, relies on the ANTLR's behavior of trying to parse
even invalid input, and mark some nodes with exception. This behavior is
somewhat a "black box", not something we should rely on, as ANTLR will
be able to parse or not invalid input depending on the complexity of the
grammar rule at hand, and the rules themselves change over time.

Hence, better and more consistent, to not mark nodes as `incomplete` but
not generate the `ON` option altogether. This PR does a deep check,
simple string check, to see if there is anything in the `ON` option at
all, if not, it does not generate it.

### Checklist

- [x] [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 f7d8bc3)
@kibanamachine kibanamachine added the backport This PR is a backport of another PR label Mar 27, 2025
@kibanamachine kibanamachine enabled auto-merge (squash) March 27, 2025 10:05
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 27, 2025

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #24 / Stateful Observability - Deployment-agnostic API integration tests observability AI Assistant when calling the title_conversation function POST /internal/observability_ai_assistant/chat/complete sends the correct system message to the LLM for the title
  • [job] [logs] FTR Configs #24 / Stateful Observability - Deployment-agnostic API integration tests observability AI Assistant when calling the title_conversation function POST /internal/observability_ai_assistant/chat/complete sends the correct system message to the LLM for the title

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 +18.0B

History

cc @vadimkibana

@kibanamachine kibanamachine merged commit 95a3a77 into elastic:8.x Mar 31, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants