Skip to content

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 <bogdan.pintea@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com> (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 <bogdan.pintea@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com>
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 <bogdan.pintea@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com>
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 <bogdan.pintea@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com>
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 <bogdan.pintea@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com>
@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

6 participants