From ca7cd215197540eff550e144a63509c0fb691d5d Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Sat, 30 Jul 2011 22:29:06 +0300 Subject: [PATCH] Failed to load uid from the index in match_all query with parent/child and _source disabled, closes #1149. --- .../action/get/TransportGetAction.java | 7 +++-- .../lucene/document/ResetFieldSelector.java | 29 +++++++++++++++++++ .../lucene/document/SingleFieldSelector.java | 8 +++-- .../mapper/internal/SourceFieldMapper.java | 26 ++--------------- .../mapper/internal/SourceFieldSelector.java | 7 +++-- .../selector/AllButSourceFieldSelector.java | 7 +++-- .../selector/FieldMappersFieldSelector.java | 13 +++++++-- .../selector/UidAndSourceFieldSelector.java | 10 ++++--- .../mapper/selector/UidFieldSelector.java | 7 +++-- .../search/fetch/FetchPhase.java | 9 +++--- 10 files changed, 78 insertions(+), 45 deletions(-) create mode 100644 modules/elasticsearch/src/main/java/org/elasticsearch/common/lucene/document/ResetFieldSelector.java diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java b/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java index b8deda469c2..880a94e0eb5 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java @@ -20,7 +20,6 @@ package org.elasticsearch.action.get; import org.apache.lucene.document.Document; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.Fieldable; import org.elasticsearch.ElasticSearchException; import org.elasticsearch.action.ActionListener; @@ -34,6 +33,7 @@ import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.common.BytesHolder; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.logging.ESLogger; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.common.lucene.uid.UidField; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.engine.Engine; @@ -164,8 +164,9 @@ public class TransportGetAction extends TransportShardSingleOperationAction fields = null; byte[] source = null; UidField.DocIdAndVersion docIdAndVersion = get.docIdAndVersion(); - FieldSelector fieldSelector = buildFieldSelectors(docMapper, gFields); + ResetFieldSelector fieldSelector = buildFieldSelectors(docMapper, gFields); if (fieldSelector != null) { + fieldSelector.reset(); Document doc; try { doc = docIdAndVersion.reader.document(docIdAndVersion.docId, fieldSelector); @@ -319,7 +320,7 @@ public class TransportGetAction extends TransportShardSingleOperationAction implements In private long compressThreshold; - private final SourceFieldSelector fieldSelector; - public SourceFieldMapper() { this(Defaults.NAME, Defaults.ENABLED, null, -1); } @@ -109,15 +106,14 @@ public class SourceFieldMapper extends AbstractFieldMapper implements In this.enabled = enabled; this.compress = compress; this.compressThreshold = compressThreshold; - this.fieldSelector = new SourceFieldSelector(names.indexName()); } public boolean enabled() { return this.enabled; } - public FieldSelector fieldSelector() { - return this.fieldSelector; + public ResetFieldSelector fieldSelector() { + return SourceFieldSelector.INSTANCE; } @Override protected Field parseCreateField(ParseContext context) throws IOException { @@ -176,22 +172,6 @@ public class SourceFieldMapper extends AbstractFieldMapper implements In return value; } - private static class SourceFieldSelector implements FieldSelector { - - private final String name; - - private SourceFieldSelector(String name) { - this.name = name; - } - - @Override public FieldSelectorResult accept(String fieldName) { - if (fieldName.equals(name)) { - return FieldSelectorResult.LOAD_AND_BREAK; - } - return FieldSelectorResult.NO_LOAD; - } - } - @Override protected String contentType() { return CONTENT_TYPE; } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldSelector.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldSelector.java index d6d25b02b4c..dbbba2c3cbc 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldSelector.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldSelector.java @@ -19,15 +19,15 @@ package org.elasticsearch.index.mapper.internal; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.FieldSelectorResult; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; /** * An optimized field selector that loads just the uid. * * @author kimchy (shay.banon) */ -public class SourceFieldSelector implements FieldSelector { +public class SourceFieldSelector implements ResetFieldSelector { public static final SourceFieldSelector INSTANCE = new SourceFieldSelector(); @@ -41,4 +41,7 @@ public class SourceFieldSelector implements FieldSelector { } return FieldSelectorResult.NO_LOAD; } + + @Override public void reset() { + } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldSelector.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldSelector.java index 1353b59583d..0662e3d9fae 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldSelector.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldSelector.java @@ -19,8 +19,8 @@ package org.elasticsearch.index.mapper.selector; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.FieldSelectorResult; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.index.mapper.internal.SourceFieldMapper; /** @@ -28,7 +28,7 @@ import org.elasticsearch.index.mapper.internal.SourceFieldMapper; * * @author kimchy (shay.banon) */ -public class AllButSourceFieldSelector implements FieldSelector { +public class AllButSourceFieldSelector implements ResetFieldSelector { public static final AllButSourceFieldSelector INSTANCE = new AllButSourceFieldSelector(); @@ -38,4 +38,7 @@ public class AllButSourceFieldSelector implements FieldSelector { } return FieldSelectorResult.LOAD; } + + @Override public void reset() { + } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/FieldMappersFieldSelector.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/FieldMappersFieldSelector.java index fd21b10b8e0..eaff64e04ab 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/FieldMappersFieldSelector.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/FieldMappersFieldSelector.java @@ -19,8 +19,8 @@ package org.elasticsearch.index.mapper.selector; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.FieldSelectorResult; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.FieldMappers; @@ -29,9 +29,11 @@ import java.util.HashSet; /** * @author kimchy (shay.banon) */ -public class FieldMappersFieldSelector implements FieldSelector { +public class FieldMappersFieldSelector implements ResetFieldSelector { private final HashSet names = new HashSet(); + private int count; + public void add(String fieldName) { names.add(fieldName); @@ -45,8 +47,15 @@ public class FieldMappersFieldSelector implements FieldSelector { @Override public FieldSelectorResult accept(String fieldName) { if (names.contains(fieldName)) { + if (++count == names.size()) { + return FieldSelectorResult.LOAD_AND_BREAK; + } return FieldSelectorResult.LOAD; } return FieldSelectorResult.NO_LOAD; } + + @Override public void reset() { + count = 0; + } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldSelector.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldSelector.java index 42d80a4991e..9d30ec512bf 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldSelector.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldSelector.java @@ -19,8 +19,8 @@ package org.elasticsearch.index.mapper.selector; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.FieldSelectorResult; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.index.mapper.internal.SourceFieldMapper; import org.elasticsearch.index.mapper.internal.UidFieldMapper; @@ -29,25 +29,27 @@ import org.elasticsearch.index.mapper.internal.UidFieldMapper; * * @author kimchy (shay.banon) */ -public class UidAndSourceFieldSelector implements FieldSelector { +public class UidAndSourceFieldSelector implements ResetFieldSelector { private int match = 0; @Override public FieldSelectorResult accept(String fieldName) { if (UidFieldMapper.NAME.equals(fieldName)) { if (++match == 2) { - match = 0; return FieldSelectorResult.LOAD_AND_BREAK; } return FieldSelectorResult.LOAD; } if (SourceFieldMapper.NAME.equals(fieldName)) { if (++match == 2) { - match = 0; return FieldSelectorResult.LOAD_AND_BREAK; } return FieldSelectorResult.LOAD; } return FieldSelectorResult.NO_LOAD; } + + @Override public void reset() { + match = 0; + } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldSelector.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldSelector.java index d999d90076b..9aa024f4edb 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldSelector.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldSelector.java @@ -19,8 +19,8 @@ package org.elasticsearch.index.mapper.selector; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.FieldSelectorResult; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.index.mapper.internal.UidFieldMapper; /** @@ -28,7 +28,7 @@ import org.elasticsearch.index.mapper.internal.UidFieldMapper; * * @author kimchy (shay.banon) */ -public class UidFieldSelector implements FieldSelector { +public class UidFieldSelector implements ResetFieldSelector { public static final UidFieldSelector INSTANCE = new UidFieldSelector(); @@ -42,4 +42,7 @@ public class UidFieldSelector implements FieldSelector { } return FieldSelectorResult.NO_LOAD; } + + @Override public void reset() { + } } \ No newline at end of file diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/modules/elasticsearch/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index 3d2b3572524..ecd22b727e3 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -20,11 +20,11 @@ package org.elasticsearch.search.fetch; import org.apache.lucene.document.Document; -import org.apache.lucene.document.FieldSelector; import org.apache.lucene.document.Fieldable; import org.apache.lucene.index.IndexReader; import org.elasticsearch.common.collect.ImmutableMap; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.lucene.document.ResetFieldSelector; import org.elasticsearch.index.Index; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.FieldMapper; @@ -81,7 +81,7 @@ public class FetchPhase implements SearchPhase { } public void execute(SearchContext context) { - FieldSelector fieldSelector = buildFieldSelectors(context); + ResetFieldSelector fieldSelector = buildFieldSelectors(context); InternalSearchHit[] hits = new InternalSearchHit[context.docIdsToLoadSize()]; for (int index = 0; index < context.docIdsToLoadSize(); index++) { @@ -181,15 +181,16 @@ public class FetchPhase implements SearchPhase { throw new FetchPhaseExecutionException(context, "Failed to load uid from the index, missing internal _uid field, current fields in the doc [" + fieldNames + "]"); } - private Document loadDocument(SearchContext context, FieldSelector fieldSelector, int docId) { + private Document loadDocument(SearchContext context, ResetFieldSelector fieldSelector, int docId) { try { + fieldSelector.reset(); return context.searcher().doc(docId, fieldSelector); } catch (IOException e) { throw new FetchPhaseExecutionException(context, "Failed to fetch doc id [" + docId + "]", e); } } - private FieldSelector buildFieldSelectors(SearchContext context) { + private ResetFieldSelector buildFieldSelectors(SearchContext context) { if (context.hasScriptFields() && !context.hasFieldNames()) { // we ask for script fields, and no field names, don't load the source return UidFieldSelector.INSTANCE;