From 4d3b6239235522df32ca25552dfc484cd6ff67ca Mon Sep 17 00:00:00 2001 From: kimchy Date: Wed, 6 Jul 2011 21:27:19 +0300 Subject: [PATCH] add include_in_parent and include_in_root explicit flags for nested cases --- .../index/mapper/DocumentMapperParser.java | 2 - .../index/mapper/ParseContext.java | 4 + .../index/mapper/object/ObjectMapper.java | 104 ++++++++++-------- .../query/type/nested/NestedQueryParser.java | 5 +- .../mapper/nested/NestedMappingTests.java | 42 ++++--- 5 files changed, 96 insertions(+), 61 deletions(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index c988c9a0919..141f9596d3a 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -84,8 +84,6 @@ public class DocumentMapperParser extends AbstractIndexComponent { .put(StringFieldMapper.CONTENT_TYPE, new StringFieldMapper.TypeParser()) .put(ObjectMapper.CONTENT_TYPE, new ObjectMapper.TypeParser()) .put(ObjectMapper.NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser()) - .put(ObjectMapper.OBJECT_AND_NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser()) - .put(ObjectMapper.ROOT_AND_NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser()) .put(MultiFieldMapper.CONTENT_TYPE, new MultiFieldMapper.TypeParser()) .put(GeoPointFieldMapper.CONTENT_TYPE, new GeoPointFieldMapper.TypeParser()) .immutableMap(); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/ParseContext.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/ParseContext.java index 6f78e7d67d2..5af3d88559f 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/ParseContext.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/ParseContext.java @@ -155,6 +155,10 @@ public class ParseContext { return this.listener; } + public Document rootDoc() { + return documents.get(0); + } + public List docs() { return this.documents; } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java index cba127eab86..d036c911b75 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java @@ -60,8 +60,6 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { public static final String CONTENT_TYPE = "object"; public static final String NESTED_CONTENT_TYPE = "nested"; - public static final String OBJECT_AND_NESTED_CONTENT_TYPE = "object_and_nested"; - public static final String ROOT_AND_NESTED_CONTENT_TYPE = "root_and_nested"; public static class Defaults { public static final boolean ENABLED = true; @@ -76,29 +74,37 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { STRICT } - public static enum Nested { - NO { - @Override public boolean isNested() { - return false; - } - }, - NESTED { - @Override public boolean isNested() { - return true; - } - }, - OBJECT_AND_NESTED { - @Override public boolean isNested() { - return true; - } - }, - ROOT_AND_NESTED { - @Override public boolean isNested() { - return true; - } - }; + public static class Nested { - public abstract boolean isNested(); + public static final Nested NO = new Nested(false, false, false); + + public static Nested newNested(boolean includeInParent, boolean includeInRoot) { + return new Nested(true, includeInParent, includeInRoot); + } + + private final boolean nested; + + private final boolean includeInParent; + + private final boolean includeInRoot; + + private Nested(boolean nested, boolean includeInParent, boolean includeInRoot) { + this.nested = nested; + this.includeInParent = includeInParent; + this.includeInRoot = includeInRoot; + } + + public boolean isNested() { + return nested; + } + + public boolean isIncludeInParent() { + return includeInParent; + } + + public boolean isIncludeInRoot() { + return includeInRoot; + } } public static class Builder extends Mapper.Builder { @@ -179,6 +185,9 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { Map objectNode = node; ObjectMapper.Builder builder = createBuilder(name); + boolean nested = false; + boolean nestedIncludeInParent = false; + boolean nestedIncludeInRoot = false; for (Map.Entry entry : objectNode.entrySet()) { String fieldName = Strings.toUnderscoreCase(entry.getKey()); Object fieldNode = entry.getValue(); @@ -195,14 +204,14 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { if (type.equals(CONTENT_TYPE)) { builder.nested = Nested.NO; } else if (type.equals(NESTED_CONTENT_TYPE)) { - builder.nested = Nested.NESTED; - } else if (type.equals(OBJECT_AND_NESTED_CONTENT_TYPE)) { - builder.nested = Nested.OBJECT_AND_NESTED; - } else if (type.equals(ROOT_AND_NESTED_CONTENT_TYPE)) { - builder.nested = Nested.ROOT_AND_NESTED; + nested = true; } else { throw new MapperParsingException("Trying to parse an object but has a different type [" + type + "] for [" + name + "]"); } + } else if (fieldName.equals("include_in_parent")) { + nestedIncludeInParent = nodeBooleanValue(fieldNode); + } else if (fieldName.equals("include_in_root")) { + nestedIncludeInRoot = nodeBooleanValue(fieldNode); } else if (fieldName.equals("enabled")) { builder.enabled(nodeBooleanValue(fieldNode)); } else if (fieldName.equals("path")) { @@ -215,6 +224,11 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { processField(builder, fieldName, fieldNode); } } + + if (nested) { + builder.nested = Nested.newNested(nestedIncludeInParent, nestedIncludeInRoot); + } + return builder; } @@ -431,8 +445,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { context.path().pathType(origPathType); if (nested.isNested()) { Document nestedDoc = context.switchDoc(restoreDoc); - if (nested == Nested.OBJECT_AND_NESTED) { - // copy over all the fields to the parent doc... + if (nested.isIncludeInParent()) { for (Fieldable field : nestedDoc.getFields()) { if (field.name().equals(UidFieldMapper.NAME) || field.name().equals(TypeFieldMapper.NAME)) { continue; @@ -440,13 +453,16 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { context.doc().add(field); } } - } else if (nested == Nested.ROOT_AND_NESTED) { - // copy over all the fields to the root doc... - for (Fieldable field : nestedDoc.getFields()) { - if (field.name().equals(UidFieldMapper.NAME) || field.name().equals(TypeFieldMapper.NAME)) { - continue; - } else { - context.docs().get(0).add(field); + } + if (nested.isIncludeInRoot()) { + // don't add it twice, if its included in parent, and we are handling the master doc... + if (!(nested.isIncludeInParent() && context.doc() == context.rootDoc())) { + for (Fieldable field : nestedDoc.getFields()) { + if (field.name().equals(UidFieldMapper.NAME) || field.name().equals(TypeFieldMapper.NAME)) { + continue; + } else { + context.rootDoc().add(field); + } } } } @@ -756,12 +772,12 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { public void toXContent(XContentBuilder builder, Params params, ToXContent custom, Mapper... additionalMappers) throws IOException { builder.startObject(name); if (nested.isNested()) { - if (nested == Nested.NESTED) { - builder.field("type", NESTED_CONTENT_TYPE); - } else if (nested == Nested.OBJECT_AND_NESTED) { - builder.field("type", OBJECT_AND_NESTED_CONTENT_TYPE); - } else if (nested == Nested.ROOT_AND_NESTED) { - builder.field("type", ROOT_AND_NESTED_CONTENT_TYPE); + builder.field("type", NESTED_CONTENT_TYPE); + if (nested.isIncludeInParent()) { + builder.field("include_in_parent", true); + } + if (nested.isIncludeInRoot()) { + builder.field("include_in_root", true); } } else if (mappers.isEmpty()) { // only write the object content type if there are no properties, otherwise, it is automatically detected builder.field("type", CONTENT_TYPE); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/type/nested/NestedQueryParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/type/nested/NestedQueryParser.java index 477c9636f36..99a9db7526f 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/type/nested/NestedQueryParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/type/nested/NestedQueryParser.java @@ -94,7 +94,7 @@ public class NestedQueryParser implements QueryParser { } } } - if (query == null || filter == null) { + if (query == null && filter == null) { throw new QueryParsingException(parseContext.index(), "[nested] requires either 'query' or 'filter' field"); } if (path == null) { @@ -105,6 +105,8 @@ public class NestedQueryParser implements QueryParser { query = new DeletionAwareConstantScoreQuery(filter); } + query.setBoost(boost); + MapperService.SmartNameObjectMapper mapper = parseContext.mapperService().smartNameObjectMapper(path); if (mapper == null) { throw new QueryParsingException(parseContext.index(), "[nested] failed to find nested object under path [" + path + "]"); @@ -136,7 +138,6 @@ public class NestedQueryParser implements QueryParser { parentFilterContext.set(currentParentFilterContext); BlockJoinQuery joinQuery = new BlockJoinQuery(query, parentFilter, scoreMode); - joinQuery.setBoost(boost); return joinQuery; } diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/nested/NestedMappingTests.java b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/nested/NestedMappingTests.java index 1ee14099b82..6fc8fb87b6f 100644 --- a/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/nested/NestedMappingTests.java +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/nested/NestedMappingTests.java @@ -42,7 +42,7 @@ public class NestedMappingTests { assertThat(docMapper.hasNestedObjects(), equalTo(true)); ObjectMapper nested1Mapper = docMapper.objectMappers().get("nested1"); - assertThat(nested1Mapper.nested(), equalTo(ObjectMapper.Nested.NESTED)); + assertThat(nested1Mapper.nested().isNested(), equalTo(true)); ParsedDocument doc = docMapper.parse("type", "1", XContentFactory.jsonBuilder() .startObject() @@ -91,9 +91,13 @@ public class NestedMappingTests { assertThat(docMapper.hasNestedObjects(), equalTo(true)); ObjectMapper nested1Mapper = docMapper.objectMappers().get("nested1"); - assertThat(nested1Mapper.nested(), equalTo(ObjectMapper.Nested.NESTED)); + assertThat(nested1Mapper.nested().isNested(), equalTo(true)); + assertThat(nested1Mapper.nested().isIncludeInParent(), equalTo(false)); + assertThat(nested1Mapper.nested().isIncludeInRoot(), equalTo(false)); ObjectMapper nested2Mapper = docMapper.objectMappers().get("nested1.nested2"); - assertThat(nested2Mapper.nested(), equalTo(ObjectMapper.Nested.NESTED)); + assertThat(nested2Mapper.nested().isNested(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInParent(), equalTo(false)); + assertThat(nested2Mapper.nested().isIncludeInRoot(), equalTo(false)); ParsedDocument doc = docMapper.parse("type", "1", XContentFactory.jsonBuilder() .startObject() @@ -130,7 +134,7 @@ public class NestedMappingTests { @Test public void multiObjectAndNested1() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") .startObject("nested1").field("type", "nested").startObject("properties") - .startObject("nested2").field("type", "object_and_nested") + .startObject("nested2").field("type", "nested").field("include_in_parent", true) .endObject().endObject() .endObject().endObject().endObject().string(); @@ -138,9 +142,13 @@ public class NestedMappingTests { assertThat(docMapper.hasNestedObjects(), equalTo(true)); ObjectMapper nested1Mapper = docMapper.objectMappers().get("nested1"); - assertThat(nested1Mapper.nested(), equalTo(ObjectMapper.Nested.NESTED)); + assertThat(nested1Mapper.nested().isNested(), equalTo(true)); + assertThat(nested1Mapper.nested().isIncludeInParent(), equalTo(false)); + assertThat(nested1Mapper.nested().isIncludeInRoot(), equalTo(false)); ObjectMapper nested2Mapper = docMapper.objectMappers().get("nested1.nested2"); - assertThat(nested2Mapper.nested(), equalTo(ObjectMapper.Nested.OBJECT_AND_NESTED)); + assertThat(nested2Mapper.nested().isNested(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInParent(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInRoot(), equalTo(false)); ParsedDocument doc = docMapper.parse("type", "1", XContentFactory.jsonBuilder() .startObject() @@ -176,8 +184,8 @@ public class NestedMappingTests { @Test public void multiObjectAndNested2() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") - .startObject("nested1").field("type", "object_and_nested").startObject("properties") - .startObject("nested2").field("type", "object_and_nested") + .startObject("nested1").field("type", "nested").field("include_in_parent", true).startObject("properties") + .startObject("nested2").field("type", "nested").field("include_in_parent", true) .endObject().endObject() .endObject().endObject().endObject().string(); @@ -185,9 +193,13 @@ public class NestedMappingTests { assertThat(docMapper.hasNestedObjects(), equalTo(true)); ObjectMapper nested1Mapper = docMapper.objectMappers().get("nested1"); - assertThat(nested1Mapper.nested(), equalTo(ObjectMapper.Nested.OBJECT_AND_NESTED)); + assertThat(nested1Mapper.nested().isNested(), equalTo(true)); + assertThat(nested1Mapper.nested().isIncludeInParent(), equalTo(true)); + assertThat(nested1Mapper.nested().isIncludeInRoot(), equalTo(false)); ObjectMapper nested2Mapper = docMapper.objectMappers().get("nested1.nested2"); - assertThat(nested2Mapper.nested(), equalTo(ObjectMapper.Nested.OBJECT_AND_NESTED)); + assertThat(nested2Mapper.nested().isNested(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInParent(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInRoot(), equalTo(false)); ParsedDocument doc = docMapper.parse("type", "1", XContentFactory.jsonBuilder() .startObject() @@ -224,7 +236,7 @@ public class NestedMappingTests { @Test public void multiRootAndNested1() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") .startObject("nested1").field("type", "nested").startObject("properties") - .startObject("nested2").field("type", "root_and_nested") + .startObject("nested2").field("type", "nested").field("include_in_root", true) .endObject().endObject() .endObject().endObject().endObject().string(); @@ -232,9 +244,13 @@ public class NestedMappingTests { assertThat(docMapper.hasNestedObjects(), equalTo(true)); ObjectMapper nested1Mapper = docMapper.objectMappers().get("nested1"); - assertThat(nested1Mapper.nested(), equalTo(ObjectMapper.Nested.NESTED)); + assertThat(nested1Mapper.nested().isNested(), equalTo(true)); + assertThat(nested1Mapper.nested().isIncludeInParent(), equalTo(false)); + assertThat(nested1Mapper.nested().isIncludeInRoot(), equalTo(false)); ObjectMapper nested2Mapper = docMapper.objectMappers().get("nested1.nested2"); - assertThat(nested2Mapper.nested(), equalTo(ObjectMapper.Nested.ROOT_AND_NESTED)); + assertThat(nested2Mapper.nested().isNested(), equalTo(true)); + assertThat(nested2Mapper.nested().isIncludeInParent(), equalTo(false)); + assertThat(nested2Mapper.nested().isIncludeInRoot(), equalTo(true)); ParsedDocument doc = docMapper.parse("type", "1", XContentFactory.jsonBuilder() .startObject()