Skip to content

Conversation

jbaiera
Copy link
Member

@jbaiera jbaiera commented Mar 26, 2025

Unmutes statement parser tests and fixes some errors in the test cases to get CI running smoothly again. Added more comments to the test cases to explain why we forcefully quote and unquote the random patterns, and also for why we manually modify the random patterns returned in some parts of the test.

Also adds further validation to join query construction logic to bar selectors from being provided in quoted index patterns. Splits test validation for the right side of the join statement into two cases, a quoted one and an unquoted one.

fixes #125536

@jbaiera jbaiera added >test-failure Triaged test failures from CI :Data Management/Data streams Data streams and their lifecycles :Analytics/ES|QL AKA ESQL v8.19.0 v9.1.0 labels Mar 26, 2025
@jbaiera jbaiera requested a review from idegtiarenko March 26, 2025 21:46
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) needs:risk Requires assignment of a risk label (low, medium, blocker) labels Mar 26, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@alex-spies alex-spies added >test Issues or PRs that are addressing/adding tests and removed >test-failure Triaged test failures from CI needs:risk Requires assignment of a risk label (low, medium, blocker) labels Mar 27, 2025
Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

Very nice PR, very helpful comments. Thanks @jbaiera !

Do we want to apply the auto-backport label before merging?

@jbaiera
Copy link
Member Author

jbaiera commented Mar 27, 2025

Do we want to apply the auto-backport label before merging?

The original change hasn't been fully backported yet (was also experiencing some related CI woes) so I plan to bring these changes in with it to keep the 8.x branch healthy.

@jbaiera jbaiera merged commit dcfe053 into elastic:main Mar 27, 2025
17 checks passed
@jbaiera jbaiera deleted the ci-fix-statement-parser-tests-selectors branch March 27, 2025 16:37
jbaiera added a commit to jbaiera/elasticsearch that referenced this pull request Mar 27, 2025
* Add validation step to quoted join query construction

* Fix double validation
@jbaiera
Copy link
Member Author

jbaiera commented Mar 27, 2025

Backporting to 8.x with #125557

elasticsearchmachine pushed a commit that referenced this pull request Mar 28, 2025
* Support index pattern selector syntax in ES|QL (#120660)

This PR updates the ES|QL grammar to include the selector portion of an index pattern. Patterns
are recombined before being sent to field caps. Field caps already supports this functionality, so
this is primarily wiring it up where needed.

* Add validation step to quoted join query construction (#125731)

* Add validation step to quoted join query construction

* Fix double validation

* Add lexer entry for METRICS commands

* Simplifying test case

* regen
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
* Add validation step to quoted join query construction

* Fix double validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL :Data Management/Data streams Data streams and their lifecycles Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] StatementParserTests testInvalidJoinPatterns failing

3 participants