Allow field mappers to retrieve fields from source. by jtibshirani · Pull Request #56928 · elastic/elasticsearch

jtibshirani · 2020-05-19T01:00:58Z

This PR adds new method FieldMapper#lookupValues(SourceLookup) that extracts and parses the source values. This lets us return values like numbers and dates in a consistent format, and also handle special data types like constant_keyword. The lookupValues method calls into parseSourceValue, which mappers can override to specify how values should be parsed.

Older draft PR: #56473

Relates to #55363.

elasticmachine · 2020-05-19T01:00:59Z

Pinging @elastic/es-search (:Search/Mapping)

jtibshirani · 2020-05-19T01:03:01Z

As part of this PR, we need to decide on the 'standard format' for each field type. I'll work to collect feedback/ discussion on this point. Here's the current proposal:

boolean: parse into boolean
binary: no change to value
completion: no change to value
constant_keyword: don't consult source, return the constant value
date: parse to date string based on mapping format
date_nanos: parse to long nanos based on mapping format
ip: collapse 0s in IPv6 addresses (to be consistent with docvalues_fields)
keyword: convert value to string, do not apply normalization
numeric types (integer, float, etc.): parse value to the right type, for example for a long field "42.3" becomes 42
range types (integer_range, etc.): parse each bound to the right type, this includes both numbers, dates, and IPs
rank_feature: convert value to float
scaled_float: convert value to float, taking scaling factor into account
text, annotated_text: convert value to string

Some notable examples where we don't parse at all:

histogram, rank_features, vector: no change to value, don't attempt to convert each component into a number. For me the complexity of digging through and standardizing these structured values outweighed the benefit.
murmur3, token_count: simply convert to string. It didn't seem practical to always recompute a hash/ token count, and I couldn't see an important use case for returning these values.

We never return a field that has been dropped through ignore_malformed. This way, the values returned for particular field will always have the same type and format. The idea is that the value is ignored in all high-level parts of the search request, including the query, aggs, and field retrieval.

Geo fields aren't handled yet, I plan to do this in a follow-up PR where we can have a dedicated discussion.

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

server/src/main/java/org/elasticsearch/index/mapper/RangeType.java

nik9000 · 2020-05-20T17:29:08Z

date: parse to long millis based on mapping format

This is one place where format is pretty important, I think. We should probably apply whatever the default "format" is for this field. Though that can totally be a follow up.

Also in a follow up, folks should be able to specify the format, I think. That way folks can say things like "give this is millis-since-epoch even if the default format is ISO8601".

I'm not sure if we need time zone support too. I know the aggs have it, but that feels like a big much at this point.

nik9000 · 2020-05-20T17:29:31Z

ip: no change to value

Should this canonicalize the IP?

jtibshirani · 2020-05-20T18:50:26Z

This is one place where format is pretty important, I think. We should probably apply whatever the default "format" is for this field. Though that can totally be a follow up.

I'll handle this now, I do think it fits best within this PR!

Also in a follow up, folks should be able to specify the format, I think. That way folks can say things like "give this is millis-since-epoch even if the default format is ISO8601".

+1, this is tracked in the meta-issue as a follow-up (#55363).

Should this canonicalize the IP?

Sorry if I'm missing something obvious, what's an example of the canonicalization you have in mind ?

nik9000 · 2020-05-20T18:59:39Z

Sorry if I'm missing something obvious, what's an example of the canonicalization you have in mind ?

I believe round tripping IPs through doc values would apply this.

nik9000 · 2020-05-20T19:00:10Z

I believe round tripping IPs through doc values would apply this.

I think it might be as simple as parsing and toStringing to make that happen though.

jtibshirani · 2020-05-20T23:04:41Z

A heads up that I rebased and force-pushed to incorporate a refactor (#56976).

astefan

LGTM.
Left few minor comments.

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

...est/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java

...ugin/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java

jtibshirani · 2020-05-28T02:38:50Z

Thanks @astefan for the review!

@nik9000 it would be great to get your review as well. Here are the changes since last time:

We now format dates using the format in the mapping. This applies to date, date_nanos, and date_range.
IPs are standardized using the utilities in InetAddresses. I don't have a strong intuition here, but it felt like a good choice since it matches what's returned in docvalue_fields.

…ource.

nik9000

LGTM! Sorry for the delay.

This PR adds new method `FieldMapper#lookupValues(SourceLookup)` that extracts and parses the source values. This lets us return values like numbers and dates in a consistent format, and also handle special data types like `constant_keyword`. The `lookupValues` method calls into `parseSourceValue`, which mappers can override to specify how values should be parsed.

jtibshirani added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types labels May 19, 2020

elasticmachine added the Team:Search Meta label for search team label May 19, 2020

jtibshirani changed the title ~~Introduce a method FieldMapper#lookupValues to retrieve fields from source.~~ Introduce FieldMapper#lookupValues to retrieve fields from source. May 19, 2020

jtibshirani force-pushed the lookup-source-value branch 3 times, most recently from 66e6018 to 00e879d Compare May 19, 2020 04:01

jtibshirani changed the title ~~Introduce FieldMapper#lookupValues to retrieve fields from source.~~ Allow field mappers to retrieve fields from source. May 19, 2020

javanna reviewed May 19, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java Outdated Show resolved Hide resolved

nik9000 reviewed May 20, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/index/mapper/RangeType.java Outdated Show resolved Hide resolved

jtibshirani requested a review from astefan May 20, 2020 18:56

jtibshirani force-pushed the field-retrieval branch from e2f206f to b09b846 Compare May 20, 2020 21:03

jtibshirani force-pushed the lookup-source-value branch from cf008a5 to fab49e7 Compare May 20, 2020 21:38

astefan approved these changes May 21, 2020

View reviewed changes

jtibshirani mentioned this pull request May 27, 2020

Search 'fields' option design + implementation #55363

Closed

10 tasks

jtibshirani force-pushed the field-retrieval branch from b09b846 to 8d64f60 Compare May 28, 2020 04:04

jtibshirani added 5 commits May 27, 2020 21:05

Introduce a method FieldMapper#lookupValues to retrieve fields from s…

6216fd8

…ource.

Ensure parseSourceValue returns strings when possible.

f44f6c3

Convert murmur3 and token_count values to strings.

a09d71f

Format dates based on the mapping format.

dcf54e9

Make sure to format dates within range types.

27e36ef

jtibshirani added 2 commits May 27, 2020 21:05

Fix indentation.

f1ac140

Standardize IP fields.

f2d259b

jtibshirani force-pushed the lookup-source-value branch from a5c3ce3 to f2d259b Compare May 28, 2020 04:16

nik9000 approved these changes May 28, 2020

View reviewed changes

jtibshirani merged this pull request into elastic:field-retrieval May 28, 2020

jtibshirani deleted the lookup-source-value branch May 28, 2020 15:11

Conversation

jtibshirani commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented May 19, 2020

Uh oh!

jtibshirani commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nik9000 commented May 20, 2020

Uh oh!

nik9000 commented May 20, 2020

Uh oh!

jtibshirani commented May 20, 2020

Uh oh!

nik9000 commented May 20, 2020

Uh oh!

nik9000 commented May 20, 2020

Uh oh!

jtibshirani commented May 20, 2020

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtibshirani commented May 28, 2020

Uh oh!

nik9000 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jtibshirani commented May 19, 2020 •

edited

Loading

jtibshirani commented May 19, 2020 •

edited

Loading