-
Notifications
You must be signed in to change notification settings - Fork 25.8k
ESQL: loading unmapped fields on synthetic _source #144112
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
Changes from 4 commits
ce8e992
cb9d2e2
b40b6dc
83ac6f6
22bbddd
6d683c0
38e6881
05ca912
612f72e
f15f4b1
be7afb8
79d3f1a
f69c522
5a1241c
b889e98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| area: ES|QL | ||
| issues: | ||
| - 143916 | ||
| pr: 144112 | ||
| summary: Loading unmapped fields on synthetic `_source` | ||
| type: bug |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -544,7 +544,7 @@ private static boolean indexSortConfigByHostName(final IndexSortConfig indexSort | |
|
|
||
| public static final TypeParser PARSER = createTypeParserWithLegacySupport(Builder::new); | ||
|
|
||
| public static final class KeywordFieldType extends TextFamilyFieldType { | ||
| public static class KeywordFieldType extends TextFamilyFieldType { | ||
|
|
||
| private static final IgnoreAbove IGNORE_ABOVE_DEFAULT = new IgnoreAbove(null, IndexMode.STANDARD); | ||
|
|
||
|
|
@@ -933,8 +933,7 @@ private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBloc | |
| return new FallbackSyntheticSourceBlockLoader.SingleValueReader<BytesRef>(nullValueBytes) { | ||
| @Override | ||
| public void convertValue(Object value, List<BytesRef> accumulator) { | ||
| String stringValue = ((BytesRef) value).utf8ToString(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you're fine to change this line rather than making KeywordFieldType non-final. ie:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to be extra defensive and avoid changing behavior if the field is not potentially unmapped, but if you're fine with it then I'm fine with it :) |
||
| String adjusted = applyIgnoreAboveAndNormalizer(stringValue); | ||
| String adjusted = applyIgnoreAboveAndNormalizer(sourceValueToString(value)); | ||
| if (adjusted != null) { | ||
| // TODO what if the value didn't change? | ||
| accumulator.add(new BytesRef(adjusted)); | ||
|
|
@@ -962,6 +961,10 @@ public void writeToBlock(List<BytesRef> values, BlockLoader.Builder blockBuilder | |
| }; | ||
| } | ||
|
|
||
| protected String sourceValueToString(Object value) { | ||
| return ((BytesRef) value).utf8ToString(); | ||
| } | ||
|
|
||
| private BlockSourceReader.LeafIteratorLookup sourceBlockLoaderLookup(BlockLoaderContext blContext) { | ||
| if (getTextSearchInfo().hasNorms()) { | ||
| return BlockSourceReader.lookupFromNorms(name()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "index": { | ||
| "mapping": { | ||
| "source": { | ||
| "mode": "synthetic" | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,23 @@ FROM partial_mapping_excluded_source_sample_data | |
| 2024-10-23T12:15:03.360Z | null | ||
| ; | ||
|
|
||
| unmappedNumericFromSyntheticSourceSingleIndex | ||
| required_capability: optional_fields_nullify_tech_preview | ||
|
|
||
| SET unmapped_fields="nullify"\; | ||
| FROM partial_mapping_synthetic_sample_data | ||
| | KEEP @timestamp, unmapped_event_duration | ||
| | SORT @timestamp DESC | ||
| | LIMIT 4 | ||
| ; | ||
|
|
||
| @timestamp:date | unmapped_event_duration:null | ||
| 2024-10-23T13:55:01.543Z | null | ||
| 2024-10-23T13:53:55.832Z | null | ||
| 2024-10-23T13:52:55.015Z | null | ||
| 2024-10-23T13:51:54.732Z | null | ||
| ; | ||
|
|
||
| fieldUnmappedInSourceButSourceDisabledSingleIndex | ||
| required_capability: optional_fields_nullify_tech_preview | ||
| required_capability: source_field_mapping | ||
|
|
@@ -86,6 +103,25 @@ nanos:date_nanos | |
| 2023-01-23T13:55:01.543123456Z | ||
| ; | ||
|
|
||
| unmappedNumericFromK8sSyntheticTsIndex | ||
| required_capability: optional_fields_nullify_tech_preview | ||
| required_capability: ts_command_v0 | ||
|
|
||
| SET unmapped_fields="nullify"\; | ||
| TS k8s_unmapped | ||
| | KEEP @timestamp, cluster, network.cost | ||
| | SORT @timestamp | ||
| | LIMIT 5 | ||
| ; | ||
|
|
||
| @timestamp:datetime | cluster:keyword | network.cost:null | ||
| 2024-05-10T00:00:29.000Z | staging | null | ||
| 2024-05-10T00:00:33.000Z | staging | null | ||
| 2024-05-10T00:00:51.000Z | prod | null | ||
| 2024-05-10T00:00:57.000Z | prod | null | ||
| 2024-05-10T00:01:25.000Z | qa | null | ||
| ; | ||
|
|
||
| keepStar | ||
| required_capability: optional_fields_nullify_tech_preview | ||
|
|
||
|
|
@@ -381,8 +417,8 @@ FROM languages | |
| | STATS c = COUNT(*) BY does_not_exist | ||
| ; | ||
|
|
||
| c:long |does_not_exist:null | ||
| 4 |null | ||
| c:long | does_not_exist:null | ||
| 4 | null | ||
| ; | ||
|
|
||
| statsGroupAliasShadowingSourceColumnNoFilter | ||
|
|
@@ -393,8 +429,8 @@ FROM languages | |
| | STATS c = COUNT(*) BY language_code = does_not_exist | ||
| ; | ||
|
|
||
| c:long |language_code:null | ||
| 4 |null | ||
| c:long | language_code:null | ||
| 4 | null | ||
| ; | ||
|
|
||
| statsGroupAliasShadowingSourceColumnWithFilter | ||
|
|
@@ -406,8 +442,8 @@ FROM languages | |
| | STATS c = COUNT(*) BY language_code = does_not_exist, language_name | ||
| ; | ||
|
|
||
| c:long |language_code:null |language_name :keyword | ||
| 1 |null |English | ||
| c:long | language_code:null | language_name :keyword | ||
| 1 | null | English | ||
| ; | ||
|
|
||
| statsGroupAliasShadowingSourceColumnWithFilterAndAggExpression | ||
|
|
@@ -420,8 +456,8 @@ FROM languages | |
| BY language_code = does_not_exist1::INTEGER + does_not_exist2::INTEGER + language_code, language_name | ||
| ; | ||
|
|
||
| c:long |language_code:integer |language_name :keyword | ||
| 1 |null |English | ||
| c:long | language_code:integer | language_name :keyword | ||
| 1 | null | English | ||
| ; | ||
|
|
||
| inlinestatsSum | ||
|
|
@@ -550,11 +586,11 @@ FROM sample_data, sample_data_str, | |
| | SORT client_ip | ||
| ; | ||
|
|
||
| client_ip:ip |foo:null |bar:keyword |baz:null | ||
| 172.21.0.5 |null |null |null | ||
| 172.21.2.113 |null |null |null | ||
| 172.21.2.162 |null |null |null | ||
| 172.21.3.15 |null |null |null | ||
| client_ip:ip | foo:null | bar:keyword | baz:null | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes were unnecessary.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course not, but I used a script to fix the alignments for the tests I did add, and it just makes it easier to align the entire file rather than align a subset of it. I can revert it if you think it's a blocker.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely not a blocker. |
||
| 172.21.0.5 | null | null | null | ||
| 172.21.2.113 | null | null | null | ||
| 172.21.2.162 | null | null | null | ||
| 172.21.3.15 | null | null | null | ||
| ; | ||
|
|
||
| forkBranchesWithDifferentSchemas | ||
|
|
@@ -792,12 +828,12 @@ TS k8s_unmapped | |
| | LIMIT 5 | ||
| ; | ||
|
|
||
| @timestamp:datetime | cluster:null | network.cost:null | ||
| 2024-05-10T00:00:29.000Z | null | null | ||
| 2024-05-10T00:00:33.000Z | null | null | ||
| 2024-05-10T00:00:51.000Z | null | null | ||
| 2024-05-10T00:00:57.000Z | null | null | ||
| 2024-05-10T00:01:25.000Z | null | null | ||
| @timestamp:datetime | cluster:keyword | network.cost:null | ||
| 2024-05-10T00:00:29.000Z | staging | null | ||
| 2024-05-10T00:00:33.000Z | staging | null | ||
| 2024-05-10T00:00:51.000Z | prod | null | ||
| 2024-05-10T00:00:57.000Z | prod | null | ||
| 2024-05-10T00:01:25.000Z | qa | null | ||
| ; | ||
|
|
||
| rateOnUnmappedTsIndex | ||
|
|
@@ -1005,7 +1041,7 @@ FROM employees | |
| | EVAL y = coalesce(bar, baz) | ||
| ; | ||
|
|
||
| avg_worked_seconds:long | birth_date:date | emp_no:integer | first_name:keyword | gender:keyword | height:double | height.float:double | height.half_float:double | height.scaled_float:double | hire_date:date | is_rehired:boolean | job_positions:keyword | languages:integer | languages.byte:integer | languages.long:long | languages.short:integer | last_name:keyword | salary:integer | salary_change:double | salary_change.int:integer | salary_change.keyword:keyword | salary_change.long:long | still_hired:boolean | foo:null | bar:null | baz:null | y:null | ||
| avg_worked_seconds:long | birth_date:date | emp_no:integer | first_name:keyword | gender:keyword | height:double | height.float:double | height.half_float:double | height.scaled_float:double | hire_date:date | is_rehired:boolean | job_positions:keyword | languages:integer | languages.byte:integer | languages.long:long | languages.short:integer | last_name:keyword | salary:integer | salary_change:double | salary_change.int:integer | salary_change.keyword:keyword | salary_change.long:long | still_hired:boolean | foo:null | bar:null | baz:null | y:null | ||
| ; | ||
|
|
||
|
|
||
|
|
@@ -1057,6 +1093,6 @@ ROW a = 12::long | |
| | DROP a | ||
| ; | ||
|
|
||
| _fork:keyword | x:long | ||
| fork1 | 12 | ||
| _fork:keyword | x:long | ||
| fork1 | 12 | ||
| ; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't change this class unless someone from the core ES team approves it. My advice is to seek approval for this from the relevant team.