Add ES|QL Locate function by tteofili · Pull Request #106899 · elastic/elasticsearch

tteofili · 2024-03-29T09:36:43Z

This adds a LOCATE function to ES|QL.

PUT testidx
{
  "mappings": {
    "properties": {
      "text_field": {
        "type": "keyword"
      }
    }
  }
}

POST testidx/_doc
{ "text_field": "hello" }

POST _query
{
  "query": """
  FROM testidx|
  EVAL s = locate(text_field, 'll')
  """
}

closes #106818

[Ticket: elastic#106818]

github-actions · 2024-03-29T09:36:54Z

Documentation preview:

✨ Changed pages

nik9000 · 2024-03-29T13:05:05Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec

 least                    |"integer|long|double|boolean|keyword|text|ip|version least(first:integer|long|double|boolean|keyword|text|ip|version, ?rest...:integer|long|double|boolean|keyword|text|ip|version)"        | first           |"integer|long|double|boolean|keyword|text|ip|version"            |""                                            |"integer|long|double|boolean|keyword|text|ip|version"                    | "Returns the minimum value from many columns."                      | false       | true | false
 left                     |"keyword left(string:keyword|text, length:integer)"                |[string, length]         |["keyword|text", "integer"]            |["The string from which to return a substring.", "The number of characters to return."]                                            |keyword                    | "Returns the substring that extracts 'length' chars from 'string' starting from the left."                      | [false, false]       | false | false
 length                   |"integer length(string:keyword|text)"                                        |string                     |"keyword|text"                 | ""                                                 |integer                    | "Returns the character length of a string."                      | false                | false | false
+locate                   |"integer locate(str:keyword|text, substr:keyword|text)"        |[str,substr]       |["keyword|text", "keyword|text"]            |["", ""]                                            |integer                    | "Returns an integer that indicates the position of a keyword substring within another string"                      | [false, false]       | false | false


I'm sorry for this test. It was a good idea, but I think it's grown to be a problem. @luigidellaquila, would you be ok if I broke this test apart into a few tests so updating it is easier? Right now it's a huge blob of text to work through.

+100, updating this test is always painful for me as well

👍 it's a good idea to break it down into smaller tests, imho.

I'm doing it. Now. Well. Maybe in an hour.

x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec

...rc/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/LocateTests.java

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Locate.java

elasticsearchmachine · 2024-04-04T13:54:41Z

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

elasticsearchmachine · 2024-04-04T13:54:42Z

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

astefan

Left one comment regarding parameter names. Also, please add more complex csv-spec tests using from command:

nested function calls locate(another_function(...)) or another_function(locate(...))
use the result of locate in stats, sort and other commands
check the warning messages it creates. In string.csv-spec look for examples (warning:Line 1:).

astefan · 2024-04-04T14:21:20Z

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Locate.java

+    )
+    public Locate(
+        Source source,
+        @Param(name = "str", type = { "keyword", "text" }, description = "An input string") Expression str,


Use the full word instead of str. There was a recent effort in having a common naming convention as much as possible in all functions. Same statement for substr below.

ChrisHegarty

LGTM. Left one small optional comment.

...rc/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/LocateTests.java

…esql_locate

nik9000 · 2024-04-05T17:57:07Z

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Locate.java

+        int codePointCount = UnicodeUtil.codePointCount(str);
+        int indexStart = indexStart(codePointCount, start);
+        String utf8ToString = str.utf8ToString();
+        return 1 + utf8ToString.indexOf(substr.utf8ToString(), utf8ToString.offsetByCodePoints(0, indexStart));


I worry about this one with characters that need two chars in utf-16.

I think you're right @nik9000, but I had to write a test to confirm. And at that point I decided to raise this PR #107172

thanks for noticing this @nik9000, and thanks @ChrisHegarty for opening #107172

This is automatically generated and should probably have been committed as part of elastic#106899.

This is automatically generated and was created as part of #106899.

leemthompo · 2024-04-10T12:05:29Z

@tteofili @nik9000 looks like we forgot to actually render all the generated docs here...

Just noticed this as I look back over history for the Kibana inline docs updates I need to do for 8.14

[Opening PR to fix]

Add Locate function

544e39f

[Ticket: elastic#106818]

elasticsearchmachine added the v8.14.0 label Mar 29, 2024

locate function

13c7fc6

nik9000 reviewed Mar 29, 2024

View reviewed changes

tteofili added 13 commits April 2, 2024 12:12

Merge branch 'main' of github.com:elastic/elasticsearch into esql_locate

b5c2a5b

move tests to AbstractFunctionTestCase

abccb58

add non covered type combination

286906d

fixed formatting

7535c6e

Merge branch 'main' of github.com:elastic/elasticsearch into esql_locate

923622b

codepoints based locate impl

74b1963

more tests, minor fix

9ae227a

use 1-indexed position, utf16 string test

a811efb

test fix

9d88812

fixed formatting

fa293d4

add optional start param

0947369

Merge branch 'main' of github.com:elastic/elasticsearch into esql_locate

ae3f11a

fixed formatting

5d247a7

tteofili marked this pull request as ready for review April 4, 2024 12:45

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Apr 4, 2024

tteofili added 4 commits April 4, 2024 15:01

utf16 tests

c9fd8e2

Merge branch 'main' of github.com:elastic/elasticsearch into esql_locate

f6ef092

utf16 tests fixed

13b4a4f

dropped test with math symbols as antlr complains

e8a6ea0

tteofili added >enhancement :Search/Search Search-related issues that do not fall into other categories and removed :Search/Search Search-related issues that do not fall into other categories labels Apr 4, 2024

avoid issues with threaded suppliers

092c2da

tteofili added the :Analytics/ES|QL AKA ESQL label Apr 4, 2024

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Apr 4, 2024

Update docs/changelog/106899.yaml

bb4b2f8

astefan reviewed Apr 4, 2024

View reviewed changes

ChrisHegarty approved these changes Apr 4, 2024

View reviewed changes

...rc/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/LocateTests.java Show resolved Hide resolved

tteofili added 4 commits April 5, 2024 09:40

added more tests, fixed param names

e3939fb

Merge branch 'esql_locate' of github.com:tteofili/elasticsearch into …

23168ef

…esql_locate

a few more tests

1ff81d6

Merge branch 'main' of github.com:elastic/elasticsearch into esql_locate

ba29a27

tteofili merged commit 54eeb62 into elastic:main Apr 5, 2024

nik9000 reviewed Apr 5, 2024

View reviewed changes

alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Apr 9, 2024

Commit LocateNoStartEvaluator

1295105

This is automatically generated and should probably have been committed as part of elastic#106899.

alex-spies mentioned this pull request Apr 9, 2024

ESQL: Commit LocateNoStartEvaluator #107270

Merged

alex-spies added a commit that referenced this pull request Apr 9, 2024

ESQL: Commit LocateNoStartEvaluator (#107270)

3674a6f

This is automatically generated and was created as part of #106899.

leemthompo mentioned this pull request Apr 10, 2024

[DOCS][ESQL] Render LOCATE function docs #107305

Merged

nik9000 mentioned this pull request Sep 10, 2024

ESQL: Functions! #98545

Open

Conversation

tteofili commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Apr 4, 2024

Uh oh!

elasticsearchmachine commented Apr 4, 2024

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisHegarty left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leemthompo commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tteofili commented Mar 29, 2024 •

edited

Loading

leemthompo commented Apr 10, 2024 •

edited

Loading