Skip to content

ESQL: Enable LOOKUP JOIN in non-snapshot builds #121193

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

Merged
merged 15 commits into from
Jan 30, 2025

Conversation

alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Jan 29, 2025

This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

  • Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
  • Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
  • Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

Relates #116208

Also fix #121185

For this, we need to remove `| JOIN ...` (without `LOOKUP`) from the
lexer/grammar to avoid enabling this in release mode as well.
Qualifiers are not yet supported. If we supported this grammar, we'd
also need to validate against usage of AS.
The tables are only required for LOOKUP_🐔, and they were sent whenever
the query contained just LOOKUP (ignoring case). Require the whole
(disabled) command to be contained in the query to avoid sending it for
LOOKUP JOIN tests.
@alex-spies alex-spies added >enhancement release highlight auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v9.0.0 v8.18.0 labels Jan 29, 2025
@alex-spies alex-spies requested a review from costin January 29, 2025 15:09
@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 Jan 29, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @alex-spies, I've created a changelog YAML for you. Note that since this PR is labelled release highlight, you need to update the changelog YAML to fill out the extended information sections.

@alex-spies alex-spies added the test-release Trigger CI checks against release build label Jan 29, 2025
alex-spies and others added 3 commits January 29, 2025 18:06
This was meant to refer to lookup_chicken, not lookup join.
Drop check for snapshot build.
@bpintea bpintea added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 29, 2025
@alex-spies alex-spies removed the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 30, 2025
@alex-spies alex-spies added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 30, 2025
@alex-spies
Copy link
Contributor Author

Ok, CI is running nearly green, but it somehow doesn't show in the PR's checks on GitHub:
image

The remaining failures in the release tests are all unrelated IMO. Here's the two build scans with the failures:

The failures are either in x-pack.inference, or in ESQL yaml tests. The former is more obviously unrelated. For the latter, the failures are in

test {p0=esql/40_tsdb/from doc with aggregate_metric_double}
FAILED	3.297s			
test {p0=esql/46_downsample/Query stats on downsampled index}
FAILED	0.900s			
test {p0=esql/40_tsdb/from index pattern unsupported counter}
FAILED	0.626s			
test {p0=esql/40_tsdb/stats on aggregate_metric_double}
FAILED	0.626s			
test {p0=esql/40_unsupported_types/unsupported}
FAILED	0.604s			

And seem to be related to this PR from yesterday #120343, which also made changes to all the failing tests. /cc @limotova . None of the tests use LOOKUP JOIN and the test failures don't seem related to the grammar changes here at all.

@alex-spies
Copy link
Contributor Author

Because CI is fine except for unrelated test failures, I will exceptionally merge this because fixing the failing, unrelated release tests will likely take a while.

@alex-spies alex-spies merged commit 70b397c into elastic:main Jan 30, 2025
15 of 19 checks passed
@alex-spies alex-spies deleted the unsnapshot_lookup_join branch January 30, 2025 15:09
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 The branch "8.19" is invalid or doesn't exist

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 121193

@alex-spies
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Jan 30, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

---------

Co-authored-by: Bogdan Pintea <[email protected]>
Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Nik Everett <[email protected]>
(cherry picked from commit 70b397c)

# Conflicts:
#	muted-tests.yml
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.interp
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.java
alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Jan 30, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

---------

Co-authored-by: Bogdan Pintea <[email protected]>
Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Nik Everett <[email protected]>
alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Jan 30, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

---------

Co-authored-by: Bogdan Pintea <[email protected]>
Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Nik Everett <[email protected]>
alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Jan 30, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

---------

Co-authored-by: Bogdan Pintea <[email protected]>
Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Nik Everett <[email protected]>
nik9000 added a commit that referenced this pull request Jan 30, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.

---------

Co-authored-by: Bogdan Pintea <[email protected]>
Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Nik Everett <[email protected]>
@limotova
Copy link
Contributor

Hi! Looked into the test failures, I think this should address it

nik9000 pushed a commit that referenced this pull request Jan 31, 2025
)

This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.
nik9000 pushed a commit that referenced this pull request Jan 31, 2025
This effectively releases LOOKUP JOIN into tech preview. Docs will follow in a separate PR.

* Enable the lexing/grammar for LOOKUP JOIN in non-snapshot builds.
* Remove the grammar for the unsupported | JOIN ... command (without LOOKUP as first keyword). The way the lexer modes work, otherwise we'd also have to enable | JOIN ... syntax on non-snapshot builds and would have to add additional validation to provide appropriate error messages.
* Remove grammar for LOOKUP JOIN index AS ... because qualifiers are not yet supported. Otherwise we'd have to put in additional validation as well to prevent such queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport pending >feature release highlight Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) test-release Trigger CI checks against release build v8.18.0 v8.19.0 v9.0.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] LogicalPlanOptimizerTests class failing
6 participants