Skip to content

Conversation

@romseygeek
Copy link
Contributor

The various geo field mappers are organised in a hierarchy that shares
parsing and indexing code. This ends up over-complicating things,
particularly when we have some mappers that accept multiple values
and others that only accept singletons. It also leads to confusing
behaviour around ignore_malformed behaviour: geo fields will ignore
all values if a single one is badly formed, while all other field mappers
will only ignore the problem value and index the rest. Finally, this
structure makes adding index-time scripts to geo_point needlessly
complex.

This commit refactors the indexing logic of the hierarchy to move the
individual value indexing logic into the concrete implementations,
and aligns the ignore_malformed behaviour with that of other mappers.

It contains two breaking changes:

  • The geo field mappers no longer check for external field values on the
    parse context. This added considerable complication to the refactored
    parse methods, and is unused anywhere in our codebase, but may
    impact plugin-based field mappers which expect to use geo fields
    as multifields
  • The geo_point field mapper now passes geohashes to its multifields
    one-by-one, instead of formatting them into a comma-delimited
    string and passing them all at once. Completion multifields using
    this as an input should still behave as normal because by default
    they would split this combined geohash string on the commas in any
    case, but keyword subfields may look different.

Fixes #69601

The various geo field mappers are organised in a hierarchy that shares
parsing and indexing code. This ends up over-complicating things,
particularly when we have some mappers that accept multiple values
and others that only accept singletons. It also leads to confusing
behaviour around ignore_malformed behaviour: geo fields will ignore
all values if a single one is badly formed, while all other field mappers
will only ignore the problem value and index the rest. Finally, this
structure makes adding index-time scripts to geo_point needlessly
complex.

This commit refactors the indexing logic of the hierarchy to move the
individual value indexing logic into the concrete implementations,
and aligns the ignore_malformed behaviour with that of other mappers.

It contains two breaking changes:

* The geo field mappers no longer check for external field values on the
  parse context. This added considerable complication to the refactored
  parse methods, and is unused anywhere in our codebase, but may
  impact plugin-based field mappers which expect to use geo fields
  as multifields
* The geo_point field mapper now passes geohashes to its multifields
  one-by-one, instead of formatting them into a comma-delimited
  string and passing them all at once. Completion multifields using
  this as an input should still behave as normal because by default
  they would split this combined geohash string on the commas in any
  case, but keyword subfields may look different.

Fixes elastic#69601
@romseygeek romseygeek self-assigned this Apr 19, 2021
@romseygeek romseygeek merged commit 416c2b1 into elastic:7.x Apr 19, 2021
@romseygeek romseygeek deleted the mapper/geo-fields-7x branch April 19, 2021 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant