Skip to content
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

Add parent-join module #24638

Merged
merged 4 commits into from
May 12, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ subprojects {
"org.elasticsearch.plugin:transport-netty4-client:${version}": ':modules:transport-netty4',
"org.elasticsearch.plugin:reindex-client:${version}": ':modules:reindex',
"org.elasticsearch.plugin:lang-mustache-client:${version}": ':modules:lang-mustache',
"org.elasticsearch.plugin:parent-join-client:${version}": ':modules:parent-join',
"org.elasticsearch.plugin:percolator-client:${version}": ':modules:percolator',
]
project.afterEvaluate {
Expand Down
3 changes: 0 additions & 3 deletions buildSrc/src/main/resources/checkstyle_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]BoolQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]BoostingQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]GeoDistanceQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]HasChildQueryBuilderTests.java" checks="LineLength" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]MoreLikeThisQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]MultiMatchQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]SpanMultiTermQueryBuilderTests.java" checks="LineLength" />
Expand Down Expand Up @@ -689,7 +688,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]MultiValueModeTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]SearchWithRejectionsIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]MissingValueIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]ChildrenIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]DateHistogramIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]DateHistogramOffsetIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]GeoDistanceIT.java" checks="LineLength" />
Expand All @@ -716,7 +714,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]basic[/\\]SearchWithRandomExceptionsIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]basic[/\\]SearchWithRandomIOExceptionsIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]basic[/\\]TransportTwoNodesSearchIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]child[/\\]ChildQuerySearchIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]child[/\\]ParentFieldLoadingIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]geo[/\\]GeoBoundingBoxIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]geo[/\\]GeoFilterIT.java" checks="LineLength" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@
import static org.elasticsearch.index.query.QueryBuilders.geoDistanceQuery;
import static org.elasticsearch.index.query.QueryBuilders.geoPolygonQuery;
import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery;
import static org.elasticsearch.index.query.QueryBuilders.hasChildQuery;
import static org.elasticsearch.index.query.QueryBuilders.hasParentQuery;
import static org.elasticsearch.index.query.QueryBuilders.idsQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
Expand Down Expand Up @@ -216,24 +214,6 @@ public void testGeoShape() throws IOException {
}
}

public void testHasChild() {
// tag::has_child
hasChildQuery(
"blog_tag", // <1>
termQuery("tag","something"), // <2>
ScoreMode.None); // <3>
// end::has_child
}

public void testHasParent() {
// tag::has_parent
hasParentQuery(
"blog", // <1>
termQuery("tag","something"), // <2>
false); // <3>
// end::has_parent
}

public void testIds() {
// tag::ids
idsQuery("my_type", "type2")
Expand Down
1 change: 1 addition & 0 deletions client/transport/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ dependencies {
compile "org.elasticsearch.plugin:reindex-client:${version}"
compile "org.elasticsearch.plugin:lang-mustache-client:${version}"
compile "org.elasticsearch.plugin:percolator-client:${version}"
compile "org.elasticsearch.plugin:parent-join-client:${version}"
testCompile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
testCompile "junit:junit:${versions.junit}"
testCompile "org.hamcrest:hamcrest-all:${versions.hamcrest}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ private InnerHitBuilder(InnerHitBuilder other) {
}
}

InnerHitBuilder(InnerHitBuilder other, QueryBuilder query, String parentChildType, boolean ignoreUnmapped) {
// TODO public for hasChild and hasParent query.
public InnerHitBuilder(InnerHitBuilder other, QueryBuilder query, String parentChildType, boolean ignoreUnmapped) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a note that these methods are for internal usage only and no one should rely on this method to remain to be available?

this(other);
this.query = query;
this.parentChildType = parentChildType;
Expand Down Expand Up @@ -751,7 +752,8 @@ public static void extractInnerHits(QueryBuilder query, Map<String, InnerHitBuil
}
}

static InnerHitBuilder rewrite(InnerHitBuilder original, QueryBuilder rewrittenQuery) {
// TODO public for hasParent and hasChild query
public static InnerHitBuilder rewrite(InnerHitBuilder original, QueryBuilder rewrittenQuery) {
if (original == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.join.BitSetProducer;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.search.join.ToParentBlockJoinQuery;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -36,6 +35,7 @@
import org.elasticsearch.index.search.NestedHelper;

import java.io.IOException;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;

Expand Down Expand Up @@ -144,7 +144,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.field(PATH_FIELD.getPreferredName(), path);
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
if (scoreMode != null) {
builder.field(SCORE_MODE_FIELD.getPreferredName(), HasChildQueryBuilder.scoreModeAsString(scoreMode));
builder.field(SCORE_MODE_FIELD.getPreferredName(), scoreModeAsString(scoreMode));
}
printBoostAndQueryName(builder);
if (innerHitBuilder != null) {
Expand Down Expand Up @@ -183,7 +183,7 @@ public static NestedQueryBuilder fromXContent(QueryParseContext parseContext) th
} else if (IGNORE_UNMAPPED_FIELD.match(currentFieldName)) {
ignoreUnmapped = parser.booleanValue();
} else if (SCORE_MODE_FIELD.match(currentFieldName)) {
scoreMode = HasChildQueryBuilder.parseScoreMode(parser.text());
scoreMode = parseScoreMode(parser.text());
} else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName)) {
queryName = parser.text();
} else {
Expand All @@ -201,6 +201,30 @@ public static NestedQueryBuilder fromXContent(QueryParseContext parseContext) th
return queryBuilder;
}

public static ScoreMode parseScoreMode(String scoreModeString) {
if ("none".equals(scoreModeString)) {
return ScoreMode.None;
} else if ("min".equals(scoreModeString)) {
return ScoreMode.Min;
} else if ("max".equals(scoreModeString)) {
return ScoreMode.Max;
} else if ("avg".equals(scoreModeString)) {
return ScoreMode.Avg;
} else if ("sum".equals(scoreModeString)) {
return ScoreMode.Total;
}
throw new IllegalArgumentException("No score mode for child query [" + scoreModeString + "] found");
}

public static String scoreModeAsString(ScoreMode scoreMode) {
if (scoreMode == ScoreMode.Total) {
// Lucene uses 'total' but 'sum' is more consistent with other elasticsearch APIs
return "sum";
} else {
return scoreMode.name().toLowerCase(Locale.ROOT);
}
}

@Override
public final String getWriteableName() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,30 +471,6 @@ public static MoreLikeThisQueryBuilder moreLikeThisQuery(Item[] likeItems) {
return moreLikeThisQuery(null, null, likeItems);
}

/**
* Constructs a new has_child query, with the child type and the query to run on the child documents. The
* results of this query are the parent docs that those child docs matched.
*
* @param type The child type.
* @param query The query.
* @param scoreMode How the scores from the children hits should be aggregated into the parent hit.
*/
public static HasChildQueryBuilder hasChildQuery(String type, QueryBuilder query, ScoreMode scoreMode) {
return new HasChildQueryBuilder(type, query, scoreMode);
}

/**
* Constructs a new parent query, with the parent type and the query to run on the parent documents. The
* results of this query are the children docs that those parent docs matched.
*
* @param type The parent type.
* @param query The query.
* @param score Whether the score from the parent hit should propagate to the child hit
*/
public static HasParentQueryBuilder hasParentQuery(String type, QueryBuilder query, boolean score) {
return new HasParentQueryBuilder(type, query, score);
}

/**
* Constructs a new parent id query that returns all child documents of the specified type that
* point to the specified id.
Expand Down
12 changes: 0 additions & 12 deletions core/src/main/java/org/elasticsearch/search/SearchModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
package org.elasticsearch.search;

import org.apache.lucene.search.BooleanQuery;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.NamedRegistry;
import org.elasticsearch.common.geo.ShapesAvailability;
import org.elasticsearch.common.geo.builders.ShapeBuilders;
Expand All @@ -45,8 +43,6 @@
import org.elasticsearch.index.query.GeoDistanceQueryBuilder;
import org.elasticsearch.index.query.GeoPolygonQueryBuilder;
import org.elasticsearch.index.query.GeoShapeQueryBuilder;
import org.elasticsearch.index.query.HasChildQueryBuilder;
import org.elasticsearch.index.query.HasParentQueryBuilder;
import org.elasticsearch.index.query.IdsQueryBuilder;
import org.elasticsearch.index.query.MatchAllQueryBuilder;
import org.elasticsearch.index.query.MatchNoneQueryBuilder;
Expand Down Expand Up @@ -103,8 +99,6 @@
import org.elasticsearch.search.aggregations.PipelineAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.adjacency.AdjacencyMatrixAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.adjacency.InternalAdjacencyMatrix;
import org.elasticsearch.search.aggregations.bucket.children.ChildrenAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.children.InternalChildren;
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder;
Expand Down Expand Up @@ -256,7 +250,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;

Expand Down Expand Up @@ -410,9 +403,6 @@ private void registerAggregations(List<SearchPlugin> plugins) {
GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new));
registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new,
ScriptedMetricAggregationBuilder::parse).addResultReader(InternalScriptedMetric::new));
registerAggregation(new AggregationSpec(ChildrenAggregationBuilder.NAME, ChildrenAggregationBuilder::new,
ChildrenAggregationBuilder::parse).addResultReader(InternalChildren::new));

registerFromPlugin(plugins, SearchPlugin::getAggregations, this::registerAggregation);
}

Expand Down Expand Up @@ -706,8 +696,6 @@ private void registerQueryParsers(List<SearchPlugin> plugins) {
MatchPhrasePrefixQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(MultiMatchQueryBuilder.NAME, MultiMatchQueryBuilder::new, MultiMatchQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(NestedQueryBuilder.NAME, NestedQueryBuilder::new, NestedQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(HasChildQueryBuilder.NAME, HasChildQueryBuilder::new, HasChildQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(HasParentQueryBuilder.NAME, HasParentQueryBuilder::new, HasParentQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(DisMaxQueryBuilder.NAME, DisMaxQueryBuilder::new, DisMaxQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(IdsQueryBuilder.NAME, IdsQueryBuilder::new, IdsQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(MatchAllQueryBuilder.NAME, MatchAllQueryBuilder::new, MatchAllQueryBuilder::fromXContent));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.aggregations.bucket.adjacency.AdjacencyMatrix;
import org.elasticsearch.search.aggregations.bucket.adjacency.AdjacencyMatrixAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.children.Children;
import org.elasticsearch.search.aggregations.bucket.children.ChildrenAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filters.Filters;
Expand Down Expand Up @@ -163,20 +161,20 @@ public static FiltersAggregationBuilder filters(String name, KeyedFilter... filt
public static FiltersAggregationBuilder filters(String name, QueryBuilder... filters) {
return new FiltersAggregationBuilder(name, filters);
}

/**
* Create a new {@link AdjacencyMatrix} aggregation with the given name.
*/
public static AdjacencyMatrixAggregationBuilder adjacencyMatrix(String name, Map<String, QueryBuilder> filters) {
return new AdjacencyMatrixAggregationBuilder(name, filters);
}
}

/**
* Create a new {@link AdjacencyMatrix} aggregation with the given name and separator
*/
public static AdjacencyMatrixAggregationBuilder adjacencyMatrix(String name, String separator, Map<String, QueryBuilder> filters) {
return new AdjacencyMatrixAggregationBuilder(name, separator, filters);
}
}

/**
* Create a new {@link Sampler} aggregation with the given name.
Expand Down Expand Up @@ -220,13 +218,6 @@ public static ReverseNestedAggregationBuilder reverseNested(String name) {
return new ReverseNestedAggregationBuilder(name);
}

/**
* Create a new {@link Children} aggregation with the given name.
*/
public static ChildrenAggregationBuilder children(String name, String childType) {
return new ChildrenAggregationBuilder(name, childType);
}

/**
* Create a new {@link GeoDistance} aggregation with the given name.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.apache.lucene.search.highlight.WeightedSpanTermExtractor;
import org.elasticsearch.common.lucene.search.function.FiltersFunctionScoreQuery;
import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery;
import org.elasticsearch.index.query.HasChildQueryBuilder;

import java.io.IOException;
import java.util.Map;
Expand Down Expand Up @@ -83,7 +82,7 @@ protected void extractUnknownQuery(Query query,
}

protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> terms) throws IOException {
if (query instanceof HasChildQueryBuilder.LateParsingQuery) {
if (isChildOrParentQuery(query.getClass())) {
// skip has_child or has_parent queries, see: https://github.com/elastic/elasticsearch/issues/14999
return;
} else if (query instanceof FunctionScoreQuery) {
Expand All @@ -94,5 +93,13 @@ protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> t
super.extract(query, boost, terms);
}
}

/**
* Workaround to detect parent/child query
*/
private static final String PARENT_CHILD_QUERY_NAME = "HasChildQueryBuilder$LateParsingQuery";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not so pretty... but I'm not sure how this check can be achieved in a better way.

private static boolean isChildOrParentQuery(Class<?> clazz) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this method public and test in the parent join module that it actually detects this query?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test for this in ChildQuerySearchIT#testHighlighHighlightersIgnoreParentChild

return clazz.getName().endsWith(PARENT_CHILD_QUERY_NAME);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe do this:

return clazz.getSimpeName().equals(PARENT_CHILD_QUERY_NAME)

?

}
}
}
Loading