Rework parent-join to not require access to DocumentMapper #63738
Conversation
romseygeek
added
:Search Foundations/Mapping
|
Pinging @elastic/es-search (:Search/Mapping) |
hell yeah! |
There was a problem hiding this comment.
This is a great change! Would it be possible to split it into smaller PRs? For instance one could be to move the mappers to parametrized field mapper, another one to replace the lookup mechanism so that looking up mappers is no longer required. Or are these two tied to each other?
|
I can probably split the parametrization out into a separate PR, although encapsulating the connection logic into the Relations class is a major part of it - I'll give it a go. |
|
I've removed the parametrization, and also undone the moving of LateParsingQuery to its own file, which ended up adding a lot of noise - I can do that in a separate PR. |
| /** | ||
| * @return a filter for child documents of a specific parent type | ||
| */ | ||
| public Query getChildrenFilter(String parentType) { |
There was a problem hiding this comment.
nit: is there a reason why this method starts with get while the two above don't?
| /** | ||
| * Get the Joiner for this context, or {@code null} if none is configured | ||
| */ | ||
| public static Joiner getJoiner(Predicate<String> isMapped, Function<String, MappedFieldType> getFieldType) { |
There was a problem hiding this comment.
shall we make this package private?
| /** | ||
| * Constructs a Joiner based on a join field and a set of relations | ||
| */ | ||
| public Joiner(String joinField, List<Relations> relations) { |
| /** | ||
| * Utility class to help build join queries and aggregations, based on a join_field | ||
| */ | ||
| public class Joiner { |
| return Objects.hash(parent, children); | ||
| } | ||
|
|
||
| static List<Relations> parse(Object node) { |
| @@ -187,6 +189,11 @@ public MappedFieldType getFieldType(String path) { | |||
| return context.getFieldType(path); | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean isFieldMapped(String field) { | |||
There was a problem hiding this comment.
you may also remove getMapper from this class , one less usage of getMapperService ;)
|
@elasticmachine update branch |
Parent joins work using a cluster of field mappers: the join field itself; a set of subfields that allow multiple relationships between parents and children to be defined; and a metadata field that acts to only allow a single join field per index to be defined. The various queries and aggregations that use this infrastructure retrieve the join field mapper via a static method and then build themselves by pulling individual relationship mappers from this main mapper. Using mappers rather than MappedFieldTypes means that we need to expose DocumentMapper at search time, which is something we are trying to avoid. This commit refactors things so that the join relations are encapsulated in a Joiner object, which lives instead on the MappedFieldType associated with the metadata join field. Rather than using the ParentJoinFieldMapper and connected ParentIdFieldMappers, we can now build queries and aggregations using this Joiner object, retrieved via the QueryShardContext or AggregationContext using a static helper method on Joiner itself.
Parent joins work using a cluster of field mappers: the join field itself;
a set of subfields that allow multiple relationships between parents and
children to be defined; and a metadata field that acts to only allow a
single join field per index to be defined. The various queries and
aggregations that use this infrastructure retrieve the join field mapper
via a static method and then build themselves by pulling individual
relationship mappers from this main mapper.
Using mappers rather than MappedFieldTypes means that we need to
expose DocumentMapper at search time, which is something we are
trying to avoid. This commit refactors things so that the join relations
are encapsulated in a Joiner object, which lives instead on the
MappedFieldType associated with the metadata join field. Rather than
using the ParentJoinFieldMapper and connected ParentIdFieldMappers,
we can now build queries and aggregations using this Joiner object,
retrieved via the QueryShardContext or AggregationContext using
a static helper method on Joiner itself. In addition, the mappers are
refactored to parametrized form, and the Builders for the metadata and
relation mappers are removed as they are always constructed directly
by the join field mapper, and never via parsable user input.