From b928e7490413a191fbe64c5288a4fc98fd42e100 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 29 Oct 2012 15:11:49 +0100 Subject: [PATCH] lucene 4: Moved from FieldSelectors to FieldVisitors. Removed BaseFieldVisitor#reset and changed SourceFieldVisitor and UidFieldVisitor to singleton to prototype. --- .../lucene/document/BaseFieldVisitor.java | 3 --- .../document/MultipleFieldsVisitor.java | 5 ----- .../lucene/document/SingleFieldVisitor.java | 5 ----- .../mapper/internal/SourceFieldMapper.java | 2 +- .../mapper/internal/SourceFieldVisitor.java | 19 +++++-------------- .../selector/AllButSourceFieldVisitor.java | 1 - .../selector/UidAndRoutingFieldVisitor.java | 7 ------- .../selector/UidAndSourceFieldVisitor.java | 7 ------- .../mapper/selector/UidFieldVisitor.java | 18 ++++++------------ .../search/fetch/FetchPhase.java | 10 ++++------ .../search/lookup/SourceLookup.java | 7 +++---- 11 files changed, 19 insertions(+), 65 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/lucene/document/BaseFieldVisitor.java b/src/main/java/org/elasticsearch/common/lucene/document/BaseFieldVisitor.java index 49b54460398..a86ce265632 100644 --- a/src/main/java/org/elasticsearch/common/lucene/document/BaseFieldVisitor.java +++ b/src/main/java/org/elasticsearch/common/lucene/document/BaseFieldVisitor.java @@ -5,9 +5,6 @@ import org.apache.lucene.index.StoredFieldVisitor; public abstract class BaseFieldVisitor extends StoredFieldVisitor { - // LUCENE 4 UPGRADE: Some field visitors need to be cleared before they can be reused. Maybe a better way. - public abstract void reset(); - // LUCENE 4 UPGRADE: Added for now to make everything work. Want to make use of Document as less as possible. public abstract Document createDocument(); diff --git a/src/main/java/org/elasticsearch/common/lucene/document/MultipleFieldsVisitor.java b/src/main/java/org/elasticsearch/common/lucene/document/MultipleFieldsVisitor.java index 8120f7c6ad4..21fd6f5006a 100644 --- a/src/main/java/org/elasticsearch/common/lucene/document/MultipleFieldsVisitor.java +++ b/src/main/java/org/elasticsearch/common/lucene/document/MultipleFieldsVisitor.java @@ -73,11 +73,6 @@ public class MultipleFieldsVisitor extends BaseFieldVisitor { return fieldsToAdd == null || fieldsToAdd.contains(fieldInfo.name) ? Status.YES : Status.NO; } - @Override - public void reset() { - doc = null; - } - @Override public Document createDocument() { return doc; diff --git a/src/main/java/org/elasticsearch/common/lucene/document/SingleFieldVisitor.java b/src/main/java/org/elasticsearch/common/lucene/document/SingleFieldVisitor.java index f4a003d685e..09ff642c8b6 100644 --- a/src/main/java/org/elasticsearch/common/lucene/document/SingleFieldVisitor.java +++ b/src/main/java/org/elasticsearch/common/lucene/document/SingleFieldVisitor.java @@ -81,9 +81,4 @@ public class SingleFieldVisitor extends BaseFieldVisitor { values.add(value); } } - - @Override - public void reset() { - values = null; - } } diff --git a/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java index e1c1d714cd1..f27d5fb75b6 100644 --- a/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java @@ -213,7 +213,7 @@ public class SourceFieldMapper extends AbstractFieldMapper implements In } public BaseFieldVisitor fieldSelector() { - return SourceFieldVisitor.INSTANCE; + return new SourceFieldVisitor(); } @Override diff --git a/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldVisitor.java b/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldVisitor.java index cb8b06e7764..844ef9d5853 100644 --- a/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldVisitor.java +++ b/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldVisitor.java @@ -32,39 +32,30 @@ import java.io.IOException; */ public class SourceFieldVisitor extends BaseFieldVisitor { - public static final SourceFieldVisitor INSTANCE = new SourceFieldVisitor(); - private static ThreadLocal loadingContext = new ThreadLocal(); - - private SourceFieldVisitor() { - } + private BytesRef source; @Override public Status needsField(FieldInfo fieldInfo) throws IOException { if (SourceFieldMapper.NAME.equals(fieldInfo.name)) { return Status.YES; } - return loadingContext.get() != null ? Status.STOP : Status.NO; + return source != null ? Status.STOP : Status.NO; } @Override public void binaryField(FieldInfo fieldInfo, byte[] value) throws IOException { - loadingContext.set(new BytesRef(value)); - } - - @Override - public void reset() { - loadingContext.remove(); + source = new BytesRef(value); } @Override public Document createDocument() { Document document = new Document(); - document.add(new StoredField("_source", loadingContext.get().utf8ToString())); + document.add(new StoredField("_source", source)); return document; } public BytesRef source() { - return loadingContext.get(); + return source; } @Override diff --git a/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldVisitor.java b/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldVisitor.java index 2a9103cf1d4..c586c7b4094 100644 --- a/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldVisitor.java +++ b/src/main/java/org/elasticsearch/index/mapper/selector/AllButSourceFieldVisitor.java @@ -28,7 +28,6 @@ import java.io.IOException; /** * A field selector that loads all fields except the source field. */ -// LUCENE 4 UPGRADE: change into singleton public class AllButSourceFieldVisitor extends MultipleFieldsVisitor { @Override diff --git a/src/main/java/org/elasticsearch/index/mapper/selector/UidAndRoutingFieldVisitor.java b/src/main/java/org/elasticsearch/index/mapper/selector/UidAndRoutingFieldVisitor.java index 7ff8e349fbf..26af9b3963d 100644 --- a/src/main/java/org/elasticsearch/index/mapper/selector/UidAndRoutingFieldVisitor.java +++ b/src/main/java/org/elasticsearch/index/mapper/selector/UidAndRoutingFieldVisitor.java @@ -31,7 +31,6 @@ import java.io.IOException; /** * An optimized field selector that loads just the uid and the routing. */ -// LUCENE 4 UPGRADE: change into singleton public class UidAndRoutingFieldVisitor extends BaseFieldVisitor { private String uid; @@ -65,12 +64,6 @@ public class UidAndRoutingFieldVisitor extends BaseFieldVisitor { } } - @Override - public void reset() { - uid = null; - routing = null; - } - public String uid() { return uid; } diff --git a/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldVisitor.java b/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldVisitor.java index 9be0fcb64b6..fc9e8008d26 100644 --- a/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldVisitor.java +++ b/src/main/java/org/elasticsearch/index/mapper/selector/UidAndSourceFieldVisitor.java @@ -32,7 +32,6 @@ import java.io.IOException; /** * An optimized field selector that loads just the uid and the source. */ -// LUCENE 4 UPGRADE: change into singleton public class UidAndSourceFieldVisitor extends BaseFieldVisitor { private String uid; @@ -57,12 +56,6 @@ public class UidAndSourceFieldVisitor extends BaseFieldVisitor { return uid != null && source != null ? Status.STOP : Status.NO; } - @Override - public void reset() { - uid = null; - source = null; - } - @Override public void binaryField(FieldInfo fieldInfo, byte[] value) throws IOException { source = new BytesRef(value); diff --git a/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldVisitor.java b/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldVisitor.java index 5307e62c8ef..10c533fc904 100644 --- a/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldVisitor.java +++ b/src/main/java/org/elasticsearch/index/mapper/selector/UidFieldVisitor.java @@ -32,15 +32,14 @@ import java.io.IOException; */ public class UidFieldVisitor extends BaseFieldVisitor { - public static final UidFieldVisitor INSTANCE = new UidFieldVisitor(); - private static ThreadLocal loadingContext = new ThreadLocal(); + private String uid; - private UidFieldVisitor() { + public UidFieldVisitor() { } @Override public void stringField(FieldInfo fieldInfo, String value) throws IOException { - loadingContext.set(value); + uid = value; } @Override @@ -48,23 +47,18 @@ public class UidFieldVisitor extends BaseFieldVisitor { if (UidFieldMapper.NAME.equals(fieldInfo.name)) { return Status.YES; } - return loadingContext.get() != null ? Status.STOP : Status.NO; - } - - @Override - public void reset() { - loadingContext.remove(); + return uid != null ? Status.STOP : Status.NO; } @Override public Document createDocument() { Document document = new Document(); - document.add(new StoredField("_uid", loadingContext.get())); + document.add(new StoredField("_uid", uid)); return document; } public String uid() { - return loadingContext.get(); + return uid; } @Override diff --git a/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index 2d7abdb29a8..e0393866ed4 100644 --- a/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -100,14 +100,14 @@ public class FetchPhase implements SearchPhase { sourceRequested = false; } else if (context.hasScriptFields()) { // we ask for script fields, and no field names, don't load the source - fieldVisitor = UidFieldVisitor.INSTANCE; + fieldVisitor = new UidFieldVisitor(); sourceRequested = false; } else { fieldVisitor = new UidAndSourceFieldVisitor(); sourceRequested = true; } } else if (context.fieldNames().isEmpty()) { - fieldVisitor = UidFieldVisitor.INSTANCE; + fieldVisitor = new UidFieldVisitor(); sourceRequested = false; } else { boolean loadAllStored = false; @@ -151,7 +151,7 @@ public class FetchPhase implements SearchPhase { } else if (extractFieldNames != null || sourceRequested) { fieldVisitor = new UidAndSourceFieldVisitor(); } else { - fieldVisitor = UidFieldVisitor.INSTANCE; + fieldVisitor = new UidFieldVisitor(); } } @@ -288,9 +288,7 @@ public class FetchPhase implements SearchPhase { private Document loadDocument(SearchContext context, @Nullable BaseFieldVisitor fieldVisitor, int docId) { try { - if (fieldVisitor != null) { - fieldVisitor.reset(); - } else { + if (fieldVisitor == null) { return context.searcher().doc(docId); } context.searcher().doc(docId, fieldVisitor); diff --git a/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java b/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java index 1b10ce779c8..a1cb2d7350a 100644 --- a/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java +++ b/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java @@ -60,8 +60,9 @@ public class SourceLookup implements Map { return source; } try { - reader.document(docId, SourceFieldVisitor.INSTANCE); - BytesRef source = SourceFieldVisitor.INSTANCE.source(); + SourceFieldVisitor sourceFieldVisitor = new SourceFieldVisitor(); + reader.document(docId, sourceFieldVisitor); + BytesRef source = sourceFieldVisitor.source(); if (source == null) { this.source = ImmutableMap.of(); } else { @@ -69,8 +70,6 @@ public class SourceLookup implements Map { } } catch (Exception e) { throw new ElasticSearchParseException("failed to parse / load source", e); - } finally { - SourceFieldVisitor.INSTANCE.reset(); } return this.source; }