Synthetic _source: ignore_malformed in geo_point#90777
Synthetic _source: ignore_malformed in geo_point#90777nik9000 wants to merge 3 commits intoelastic:mainfrom
Conversation
|
I'm not super happy with how this turned out. It feels very shaky. Very wobbly. It works because we use |
This adds support for `geo_point` fields configured with `ignore_malformed` for synthetic `_source`. `geo_point` fields are the first that support object or array valued malformed fields. To support *that* we copy complex fields while we read them.
| throw new MapperParsingException("failed to parse field [" + fieldType().name() + "] of type [" + contentType() + "]", e); | ||
| } | ||
| }); | ||
| }, ignoreMalformed() && context.isSyntheticSource()); |
There was a problem hiding this comment.
Rather than passing this boolean all the way down, can we not wrap the context parser here? I think that would make this feel less fragile.
There was a problem hiding this comment.
The complexity here is because of arrays. For arrays, we will ignore only the ones that are malformed but keep the ones that are correctly parsed. So we need to generate one stored field value for each malformed element of the array.
Unfortunately array parsing is tricky because we accept values like [0,0] so array parsing is done on the point parser.
That means as well we cannot wrap the parser here.
There was a problem hiding this comment.
Yeah. I tried wrapping here. It doesn't work well for the reasons @iverase mentioned. Partly because of the tricky formats we support and partly because we want to do ignore_malformed for each individual point.
I suppose the thing that I like the least is this whole "wrap the parser to support objects and it's always an object" thing. It just feels super leaky and weird. OTOH, what we're doing is tricky so maybe it has to leak to some degree.
There was a problem hiding this comment.
The value of saveSyntheticSourceForMalformed is always going to be the same for a given mapper, right? So I think my worries would be assuaged a bit if we added it as a parameter to the PointParser constructor rather than passing the same boolean value through half-a-dozen intervening functions for each document.
ignoreMalformed on PointParser is a bit weird anyway. The Cartesian implementation uses it to ignore validation, but the GeoPoint version uses it to attempt normalisation (but then throws an error anyway if the values can't be normalised).
This adds support for
geo_pointfields configured withignore_malformedfor synthetic_source.geo_pointfields are the first that support object or array valued malformed fields. To support that we copy complex fields while we read them.