ESQL: Expose "_ignored" metadata field #108770

ivancea · 2024-05-17T11:59:23Z

Expose "_ignored" metadata field in ESQL queries

Fixes #108324

github-actions · 2024-05-17T11:59:35Z

Documentation preview:

✨ Changed pages

elasticsearchmachine · 2024-05-17T12:00:15Z

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

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

ivancea · 2024-05-17T13:05:04Z

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/MetadataAttribute.java

@@ -28,6 +29,8 @@ public class MetadataAttribute extends TypedAttribute {
        tuple(DataTypes.KEYWORD, true),
        IdFieldMapper.NAME,
        tuple(DataTypes.KEYWORD, false), // actually searchable, but fielddata access on the _id field is disallowed by default
+        IgnoredFieldMapper.NAME,
+        tuple(DataTypes.KEYWORD, false),


Should this be "searchable"? I'm not sure of the implications

I'm unsure too! Maybe @astefan knows.

After checking the _field_caps endpoint, it says it's searchable. Changed it; but still I don't know the implications, or how to test whether it should be that or not 👀

Should this be "searchable"? I'm not sure of the implications

If it's searchable, it'll be pushed down in a filter, if possible (WHERE _ignored == "some field"). However, this isn't always doable even if searchable and pushable (see _id), but it is in this case (and your tests cover that).

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

nik9000 · 2024-05-17T16:04:49Z

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

+
+count:long
+0
+;


I might just do all of these in a yaml test - it's possible for us to make _ignored work well in a csv test, but I'm not sure it's worth it. The yaml tests do all of the same bwc work as the csv tests and this feels enough like you want to roll your own index anyway.

ivancea · 2024-05-20T11:23:26Z

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/140_metadata.yml

FYI: Named this "140_metadata". It would be nice to me if this was something like "141_metadata_ignored", but as there's no plain metadata yet, going with this, unless there are more suggestions 👀

I haven't a clue why we use the number prefixes. We don't actually order the execution or anything.

140_metadata is fine - we can rename them later when we add more. Or not if we don't. There isn't any risk involved.

ivancea · 2024-05-20T11:24:26Z

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/140_metadata.yml

+        refresh: true
+        body:
+          - { index: { } }
+          - { case: "ok", integer: 10, keyword: "ok" }


FYI: Used a "case" field to reuse the index and avoid sorting flakyness/errors

These are the names of the test case documents? Makes sense.

ivancea · 2024-05-20T11:25:11Z

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

With the addition of the yaml test, I wonder if we should keep this one (?)

Yeah, I'm not sure there's much point in keeping the _ignore csv tests. They just end up with null all the time. I'd just run them all in yaml tests. Probably worth porting one or two more of these fun cases to the yaml tests too - like the filtering and aggregation. Just out of paranoia. It should work, but these tests are more about making sure we don't accidentally break it in nine months without realizing it.

Added the interesting tests here (where, group, aggr), and removed the csv ones

elasticsearchmachine · 2024-05-20T11:59:49Z

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

nik9000 · 2024-05-20T17:06:42Z

x-pack/plugin/esql/qa/server/multi-node/build.gradle

@@ -18,7 +18,7 @@ tasks.named('javaRestTest') {

 restResources {
  restApi {
-    include '_common', 'bulk', 'get', 'indices', 'esql', 'xpack', 'enrich', 'cluster'
+    include '_common', 'bulk', 'get', 'indices', 'esql', 'xpack', 'enrich', 'cluster', 'capabilities'


Yeah, I noticed these were required by the capabilities support in yaml.

nik9000 · 2024-05-20T17:08:05Z

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

Yeah, I'm not sure there's much point in keeping the _ignore csv tests. They just end up with null all the time. I'd just run them all in yaml tests. Probably worth porting one or two more of these fun cases to the yaml tests too - like the filtering and aggregation. Just out of paranoia. It should work, but these tests are more about making sure we don't accidentally break it in nine months without realizing it.

nik9000 · 2024-05-20T17:09:01Z

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/140_metadata.yml

140_metadata is fine - we can rename them later when we add more. Or not if we don't. There isn't any risk involved.

nik9000 · 2024-05-20T17:09:44Z

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/140_metadata.yml

+        refresh: true
+        body:
+          - { index: { } }
+          - { case: "ok", integer: 10, keyword: "ok" }


These are the names of the test case documents? Makes sense.

This reverts commit 47370a1.

Expose "_ignored" metadata field in ESQL queries

Reverting after tests failure: https://gradle-enterprise.elastic.co/s/dpi2eib2x2fj2 Reverts #108770

Expose "_ignored" metadata field in ESQL queries. This is the same code merged here: #108770 Which got reverted here: #108864 It was reverted because of a test failure: https://gradle-enterprise.elastic.co/s/dpi2eib2x2fj2

ivancea added 3 commits May 16, 2024 19:52

Allow fetching _ignored field on Lucene +9.10 nodes

5110232

Added support for legacy ignored fields

3da1543

Added capability and csv tests

6178b28

ivancea added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v8.15.0 >feature labels May 17, 2024

Update docs/changelog/108770.yaml

6d95f59

ivancea added 2 commits May 17, 2024 14:02

Merge branch 'main' into feat/esql-ignored-metadata-field

16c3bbc

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Fixed csv test type and value

c5b1020

ivancea commented May 17, 2024

View reviewed changes

ivancea added 4 commits May 17, 2024 15:12

CSV tests cleanup and capabilities readme

43c62a9

Fixed count test for nulls

27476a9

Use capability-per-line syntax

cbb7341

Merge branch 'main' into feat/esql-ignored-metadata-field

b2e9fd6

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

ivancea requested a review from nik9000 May 17, 2024 15:08

ivancea added 2 commits May 17, 2024 17:13

Standardize PR by using always NAme instead of name()

3e86f10

Make field searchable based on _field_caps

0f91ccd

nik9000 reviewed May 17, 2024

View reviewed changes

ivancea added 2 commits May 20, 2024 10:27

Merge branch 'elastic:main' into feat/esql-ignored-metadata-field

fc1e52b

Added rest test for ignored fields

95738cc

ivancea commented May 20, 2024

View reviewed changes

Added base metadata capability check

4298c0d

ivancea marked this pull request as ready for review May 20, 2024 11:59

Add capabilities rest resource to esql qa projects

1b1154b

ivancea requested a review from a team as a code owner May 20, 2024 13:53

ivancea requested a review from nik9000 May 20, 2024 14:42

nik9000 reviewed May 20, 2024

View reviewed changes

ivancea added 2 commits May 21, 2024 11:44

Moved aggregations csv tests to yml

41fad37

Added where test on _ignoreed field

5b01521

ivancea added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label May 21, 2024

elasticsearchmachine merged commit 47370a1 into elastic:main May 21, 2024
15 checks passed

ivancea deleted the feat/esql-ignored-metadata-field branch May 21, 2024 11:32

ivancea added a commit that referenced this pull request May 21, 2024

Revert "ESQL: Expose "_ignored" metadata field (#108770)"

ae60dbe

This reverts commit 47370a1.

ivancea mentioned this pull request May 21, 2024

Revert "ESQL: Expose "_ignored" metadata field" #108864

Merged

jedrazb pushed a commit to jedrazb/elasticsearch that referenced this pull request May 21, 2024

ESQL: Expose "_ignored" metadata field (elastic#108770)

b42c906

Expose "_ignored" metadata field in ESQL queries

elasticsearchmachine pushed a commit that referenced this pull request May 21, 2024

Revert "ESQL: Expose "_ignored" metadata field" (#108864)

04f1fce

Reverting after tests failure: https://gradle-enterprise.elastic.co/s/dpi2eib2x2fj2 Reverts #108770

ivancea mentioned this pull request May 21, 2024

Reapply "ESQL: Expose "_ignored" metadata field" (#108864) #108871

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESQL: Expose "_ignored" metadata field #108770

ESQL: Expose "_ignored" metadata field #108770

ivancea commented May 17, 2024 •

edited

github-actions bot commented May 17, 2024

elasticsearchmachine commented May 17, 2024

ivancea May 17, 2024

nik9000 May 17, 2024

ivancea May 17, 2024 •

edited

bpintea May 21, 2024

nik9000 May 17, 2024

ivancea May 20, 2024

nik9000 May 20, 2024

nik9000 May 20, 2024

ivancea May 20, 2024

nik9000 May 20, 2024

ivancea May 20, 2024

nik9000 May 20, 2024

ivancea May 21, 2024

elasticsearchmachine commented May 20, 2024

nik9000 May 20, 2024

nik9000 May 20, 2024

nik9000 May 20, 2024

nik9000 May 20, 2024

ESQL: Expose "_ignored" metadata field #108770

ESQL: Expose "_ignored" metadata field #108770

Conversation

ivancea commented May 17, 2024 • edited

github-actions bot commented May 17, 2024

elasticsearchmachine commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivancea May 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticsearchmachine commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivancea commented May 17, 2024 •

edited

ivancea May 17, 2024 •

edited