Skip to content

Remove snapshot build restriction for match and qstr functions #114482

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 111 commits into from
Oct 15, 2024
Merged

Remove snapshot build restriction for match and qstr functions #114482

merged 111 commits into from
Oct 15, 2024

Conversation

carlosdelest
Copy link
Member

@carlosdelest carlosdelest commented Oct 10, 2024 •
edited
Loading

Includes match and qstr functions in non-snapshot buids, in order to provide tech preview access for users.

This removes checks on tests and lexer / parser grammar

carlosdelest and others added 30 commits September 20, 2024 14:25
@carlosdelest
First version
ab5852c
@carlosdelest
Wire up for capabilities and function registry
35573a3
@carlosdelest
Minor text message change
b2fc887
@carlosdelest
Changed hierarchy to allow for more expressions besides query
fdbd226
@carlosdelest
Change name to matchstr in registry
e4c1fbe
@carlosdelest
Added VerifierTests for matchstr
9c93412
@carlosdelest
Fix TypeResolution method
f48d648
@carlosdelest
Accept only a field as a first argument
58e1eee
@carlosdelest
Add CSV tests
242d993
@carlosdelest
Modified type errors to include source and not function name
11b59f0
@carlosdelest
Don't do CSV tests for match function
0acd604
@carlosdelest
Add MatchFunctionTests
7957382
@carlosdelest
Add generated docs
3b28cc9
@carlosdelest
Make PR easier to read
c9449c7
@carlosdelest
Minor refactorings for better javadoc and subclass structure for Full…
1fa0c60 
…TextFunctions
@carlosdelest
Spotless
f88bf30
@carlosdelest
Check function defined in tests
880b4e6
@carlosdelest
Merge branch 'main' into carlosdelest/esql-match-function
5718d68 
# Conflicts:
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@carlosdelest
Rename match operator to matchstr
e23eef3
@carlosdelest
Rename matchstr function to match
862374d
@carlosdelest
Rename matchstr function to match
f9371b1
@carlosdelest
Rename match operator to matchstr
0a05d5b
@carlosdelest
Don't run REST compat test for match operator
6601dfa
@carlosdelest
Revert "Rename match operator to matchstr"
dd6df12 
This reverts commit 0a05d5b.
@carlosdelest
Revert "Rename match operator to matchstr"
16d88b0 
This reverts commit e23eef3
@carlosdelest
Adds MATCH as a valid function identifier to make it compatible with …
9e32c8e 
…match operator
@carlosdelest
Merge branch 'main' into carlosdelest/esql-match-function
3297ea5 
# Conflicts:
#	x-pack/plugin/build.gradle
@carlosdelest
Removed unnecessary code
953b276
@carlosdelest
Final removal of previous renames
2877688
@carlosdelest
Update docs/changelog/113374.yaml
274c94f
@carlosdelest carlosdelest marked this pull request as ready for review October 14, 2024 08:49
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @carlosdelest, I've created a changelog YAML for you.

@carlosdelest carlosdelest added the ES|QL-ui Impacts ES|QL UI label Oct 14, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/kibana-esql (ES|QL-ui)

@carlosdelest
Copy link
Member Author

Flagging with ESQL-UI as two new functions will be available: match() and qstr().

ChrisHegarty
ChrisHegarty approved these changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

The removal of the snapshot flag looks correct to me. Where do we say in docs that the functions are Tech Preview? Is it covered by "preview" : true ?

@carlosdelest carlosdelest added the test-release Trigger CI checks against release build label Oct 14, 2024
@carlosdelest
Copy link
Member Author

@elasticmachine test this please

@drewdaemon
Copy link
Contributor

Am I correct in understanding that match is only applicable in a WHERE command?

@carlosdelest
Copy link
Member Author

The release test CI failure us caused by an unrelated test failure: https://buildkite.com/elastic/elasticsearch-pull-request/builds/36049#01928140-b187-4821-bd89-e4c08c244e72/6-2132

This failure is tracked in #114135

@carlosdelest
Copy link
Member Author

Am I correct in understanding that match is only applicable in a WHERE command?

@drewdaemon correct. Both match and qstr functions can only be used as part of WHERE.

@carlosdelest
Copy link
Member Author

The removal of the snapshot flag looks correct to me. Where do we say in docs that the functions are Tech Preview? Is it covered by "preview" : true ?

@ChrisHegarty yes - you can see in the generated docs that there's a reference to the tech preview because of that.

@carlosdelest carlosdelest merged commit 7ad1a0c into elastic:main Oct 15, 2024
15 of 17 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

The backport operation could not be completed due to the following error:

An unexpected error occurred when attempting to backport this PR.

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

carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Oct 15, 2024
@carlosdelest
Remove snapshot build restriction for match and qstr functions (elast…
80b420a 
…ic#114482)

(cherry picked from commit 7ad1a0c)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java
@carlosdelest carlosdelest mentioned this pull request Oct 15, 2024
Merged
@carlosdelest
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation

carlosdelest added a commit that referenced this pull request Oct 15, 2024
@carlosdelest
Remove snapshot build restriction for match and qstr functions (#114482…
581894a 
…) (#114793)
davidkyle pushed a commit that referenced this pull request Oct 15, 2024
@carlosdelest @davidkyle
Remove snapshot build restriction for match and qstr functions (#114482)
0b3e705
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
@carlosdelest @georgewallace
Remove snapshot build restriction for match and qstr functions (elast…
2961658 
…ic#114482)
@carlosdelest carlosdelest mentioned this pull request Nov 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ioanatia ioanatia ioanatia approved these changes

@ChrisHegarty ChrisHegarty ChrisHegarty approved these changes

@tteofili tteofili Awaiting requested review from tteofili

@afoucret afoucret Awaiting requested review from afoucret

@fang-xing-esql fang-xing-esql Awaiting requested review from fang-xing-esql

Assignees

No one assigned

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged backport pending ES|QL-ui Impacts ES|QL UI >feature Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) test-release Trigger CI checks against release build v8.16.0 v9.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@carlosdelest @elasticsearchmachine @drewdaemon @ioanatia @ChrisHegarty @elasticmachine