Skip to content

MappedFieldType should not extend FieldType #57666

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.

Sign up for GitHub

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

Merged
merged 21 commits into from
Jun 15, 2020
Merged

MappedFieldType should not extend FieldType #57666

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
23a35eb
Cut 1: /server compiles, mapping tests pass
romseygeek Jun 3, 2020
ac2cc4a
server/ tests passing
romseygeek Jun 4, 2020
b18c1b9
Merge remote-tracking branch 'origin/master' into mapper/fieldtype
romseygeek Jun 4, 2020
344520b
compilation
romseygeek Jun 7, 2020
b26fd80
Merge remote-tracking branch 'origin/master' into mapper/fieldtype
romseygeek Jun 7, 2020
f331482
mapper-extras, analytics
romseygeek Jun 8, 2020
a146b6f
percolator
romseygeek Jun 8, 2020
fc79105
server integtests
romseygeek Jun 8, 2020
3772c31
checkstyle
romseygeek Jun 8, 2020
bb8a244
flattened; precommit
romseygeek Jun 8, 2020
5e6ce42
tests
romseygeek Jun 9, 2020
50d43a6
tests
romseygeek Jun 9, 2020
9b6ebdf
tests
romseygeek Jun 9, 2020
f71d49e
line length
romseygeek Jun 9, 2020
a5bbeee
moar
romseygeek Jun 9, 2020
39468c0
murmur3
romseygeek Jun 9, 2020
6816f5f
Merge remote-tracking branch 'origin/master' into mapper/fieldtype
romseygeek Jun 9, 2020
a9e36a7
percolator
romseygeek Jun 10, 2020
f97840e
Merge remote-tracking branch 'origin/master' into mapper/fieldtype
romseygeek Jun 15, 2020
fd0e5e1
feedback
romseygeek Jun 15, 2020
0ac318d
null check for booleans
romseygeek Jun 15, 2020
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
4 changes: 2 additions & 2 deletions .../src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.analysis.TokenFilterFactory;
import org.elasticsearch.index.analysis.TokenizerFactory;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.StringFieldType;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -179,7 +179,7 @@ private static Analyzer getAnalyzer(AnalyzeAction.Request request, AnalysisRegis
}
MappedFieldType fieldType = indexService.mapperService().fieldType(request.field());
if (fieldType != null) {
if (fieldType.tokenized() || fieldType instanceof KeywordFieldMapper.KeywordFieldType) {
Copy link
Member

Choose a reason for hiding this comment

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

I super hate all of these instanceofs!

Copy link
Member

Choose a reason for hiding this comment

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

But you are making it better.

if (fieldType instanceof StringFieldType) {
return fieldType.indexAnalyzer();
} else {
throw new IllegalArgumentException("Can't process field [" + request.field() +
Expand Down
74 changes: 26 additions & 48 deletions server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
package org.elasticsearch.index.mapper;

import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
Expand Down Expand Up @@ -58,6 +58,11 @@ public static class Names {
public static class Defaults {
public static final Explicit<Boolean> IGNORE_MALFORMED = new Explicit<>(false, false);
public static final Explicit<Boolean> IGNORE_Z_VALUE = new Explicit<>(true, false);
public static final FieldType FIELD_TYPE = new FieldType();
static {
FIELD_TYPE.setStored(false);
FIELD_TYPE.freeze();
}
}

/**
Expand All @@ -76,18 +81,19 @@ public interface Parser<Parsed> {
Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException;
}

public abstract static class Builder<T extends Builder, FT extends AbstractGeometryFieldType>
public abstract static class Builder<T extends Builder<T, FT>, FT extends AbstractGeometryFieldType>
extends FieldMapper.Builder<T> {
protected Boolean ignoreMalformed;
protected Boolean ignoreZValue;
protected boolean indexed = true;

public Builder(String name, MappedFieldType fieldType, MappedFieldType defaultFieldType) {
super(name, fieldType, defaultFieldType);
public Builder(String name, FieldType fieldType) {
super(name, fieldType);
}

public Builder(String name, MappedFieldType fieldType, MappedFieldType defaultFieldType, boolean ignoreMalformed,
public Builder(String name, FieldType fieldType, boolean ignoreMalformed,
boolean ignoreZValue) {
super(name, fieldType, defaultFieldType);
super(name, fieldType);
this.ignoreMalformed = ignoreMalformed;
this.ignoreZValue = ignoreZValue;
}
Expand Down Expand Up @@ -132,30 +138,6 @@ public Builder ignoreZValue(final boolean ignoreZValue) {
this.ignoreZValue = ignoreZValue;
return this;
}

@Override
protected void setupFieldType(BuilderContext context) {
super.setupFieldType(context);

// field mapper handles this at build time
// but prefix tree strategies require a name, so throw a similar exception
if (name().isEmpty()) {
throw new IllegalArgumentException("name cannot be empty string");
}

setGeometryParser();
setGeometryIndexer(fieldType());
setGeometryQueryBuilder(fieldType());
}

@Override
public FT fieldType() {
return (FT)fieldType;
}

protected abstract void setGeometryParser();
protected abstract void setGeometryIndexer(FT fieldType);
protected abstract void setGeometryQueryBuilder(FT fieldType);
}

public abstract static class TypeParser<T extends Builder> implements Mapper.TypeParser {
Expand Down Expand Up @@ -202,12 +184,8 @@ public abstract static class AbstractGeometryFieldType<Parsed, Processed> extend
protected Parser<Parsed> geometryParser;
protected QueryProcessor geometryQueryBuilder;

protected AbstractGeometryFieldType() {
setIndexOptions(IndexOptions.DOCS);
setTokenized(false);
setStored(false);
setStoreTermVectors(false);
setOmitNorms(true);
protected AbstractGeometryFieldType(String name, boolean indexed, boolean hasDocValues, Map<String, String> meta) {
super(name, indexed, hasDocValues, meta);
}

protected AbstractGeometryFieldType(AbstractGeometryFieldType ref) {
Expand All @@ -226,7 +204,7 @@ protected Indexer<Parsed, Processed> geometryIndexer() {
return geometryIndexer;
}

public void setGeometryParser(Parser geometryParser) {
public void setGeometryParser(Parser<Parsed> geometryParser) {
this.geometryParser = geometryParser;
}

Expand Down Expand Up @@ -271,10 +249,10 @@ public Query termQuery(Object value, QueryShardContext context) {
protected Explicit<Boolean> ignoreMalformed;
protected Explicit<Boolean> ignoreZValue;

protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType,
protected AbstractGeometryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
Settings indexSettings, Explicit<Boolean> ignoreMalformed,
Explicit<Boolean> ignoreZValue, MultiFields multiFields, CopyTo copyTo) {
super(simpleName, fieldType, defaultFieldType, indexSettings, multiFields, copyTo);
super(simpleName, fieldType, mappedFieldType, indexSettings, multiFields, copyTo);
this.ignoreMalformed = ignoreMalformed;
this.ignoreZValue = ignoreZValue;
}
Expand All @@ -293,7 +271,7 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {

@Override
public AbstractGeometryFieldType fieldType() {
return (AbstractGeometryFieldType)fieldType;
return (AbstractGeometryFieldType)mappedFieldType;
}

@Override
Expand All @@ -308,10 +286,10 @@ protected void parseCreateField(ParseContext context) throws IOException {
/** parsing logic for geometry indexing */
@Override
public void parse(ParseContext context) throws IOException {
AbstractGeometryFieldMapper.AbstractGeometryFieldType fieldType = fieldType();
AbstractGeometryFieldMapper.AbstractGeometryFieldType mappedFieldType = fieldType();

@SuppressWarnings("unchecked") Indexer<Parsed, Processed> geometryIndexer = fieldType.geometryIndexer();
@SuppressWarnings("unchecked") Parser<Parsed> geometryParser = fieldType.geometryParser();
@SuppressWarnings("unchecked") Indexer<Parsed, Processed> geometryIndexer = mappedFieldType.geometryIndexer();
@SuppressWarnings("unchecked") Parser<Parsed> geometryParser = mappedFieldType.geometryParser();
try {
Processed shape = context.parseExternalValue(geometryIndexer.processedClass());
if (shape == null) {
Expand All @@ -323,13 +301,13 @@ public void parse(ParseContext context) throws IOException {
}

List<IndexableField> fields = new ArrayList<>();
if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.hasDocValues()) {
if (mappedFieldType.isSearchable() || mappedFieldType.hasDocValues()) {
Copy link
Member

Choose a reason for hiding this comment

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

what happens with these checks once isSearchable returns true yet there is no index? Do we need to distinguish between the two?

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 will need to distinguish between them in the future, yes. This is something of a shim though - the plan is to eventually move everything to parametrized mappers (see #58663) which will change how this works again.

fields.addAll(geometryIndexer.indexShape(context, shape));
}

// indexed:
List<IndexableField> indexedFields = new ArrayList<>();
if (fieldType.indexOptions() != IndexOptions.NONE) {
if (mappedFieldType.isSearchable()) {
indexedFields.addAll(fields);
}
// stored:
Expand All @@ -338,8 +316,8 @@ public void parse(ParseContext context) throws IOException {
}
// docValues:
if (fieldType().hasDocValues()) {
addDocValuesFields(fieldType.name(), shape, fields, context);
} else if (fieldType.stored() || fieldType.indexOptions() != IndexOptions.NONE) {
addDocValuesFields(mappedFieldType.name(), shape, fields, context);
} else if (fieldType.stored() || fieldType().isSearchable()) {
createFieldNamesField(context);
}

Expand All @@ -355,7 +333,7 @@ public void parse(ParseContext context) throws IOException {
throw new MapperParsingException("failed to parse field [{}] of type [{}]", e, fieldType().name(),
fieldType().typeName());
}
context.addIgnoredField(fieldType.name());
context.addIgnoredField(mappedFieldType.name());
}
}

Expand Down
75 changes: 42 additions & 33 deletions server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.index.mapper;

import org.apache.lucene.document.FieldType;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.ParseField;
Expand All @@ -44,37 +45,45 @@ public static class Names extends AbstractGeometryFieldMapper.Names {
public static final ParseField NULL_VALUE = new ParseField("null_value");
}

public abstract static class Builder<T extends Builder,
public static final FieldType DEFAULT_FIELD_TYPE = new FieldType();
static {
DEFAULT_FIELD_TYPE.setDimensions(2, Integer.BYTES);
DEFAULT_FIELD_TYPE.setStored(false);
DEFAULT_FIELD_TYPE.freeze();
}

public abstract static class Builder<T extends Builder<T, FT>,
FT extends AbstractPointGeometryFieldType> extends AbstractGeometryFieldMapper.Builder<T, FT> {
public Builder(String name, MappedFieldType fieldType, MappedFieldType defaultFieldType) {
super(name, fieldType, defaultFieldType);
}

public abstract AbstractPointGeometryFieldMapper build(BuilderContext context, String simpleName, MappedFieldType fieldType,
MappedFieldType defaultFieldType, Settings indexSettings,
MultiFields multiFields, Explicit<Boolean> ignoreMalformed, Explicit<Boolean> ignoreZValue,
CopyTo copyTo);
private ParsedPoint nullValue;

public Builder(String name, FieldType fieldType) {
super(name, fieldType);
}

@Override
public AbstractPointGeometryFieldMapper build(BuilderContext context) {
return build(context, name, fieldType, defaultFieldType, context.indexSettings(),
multiFieldsBuilder.build(this, context), ignoreMalformed(context),
ignoreZValue(context), copyTo);
public void setNullValue(ParsedPoint nullValue) {
this.nullValue = nullValue;
}

public abstract AbstractPointGeometryFieldMapper build(BuilderContext context, String simpleName, FieldType fieldType,
Settings indexSettings, MultiFields multiFields,
Explicit<Boolean> ignoreMalformed,
Explicit<Boolean> ignoreZValue,
ParsedPoint nullValue, CopyTo copyTo);


@Override
public FT fieldType() {
return (FT)fieldType;
public AbstractPointGeometryFieldMapper build(BuilderContext context) {
return build(context, name, fieldType, context.indexSettings(),
multiFieldsBuilder.build(this, context), ignoreMalformed(context),
ignoreZValue(context), nullValue, copyTo);
}
}

public abstract static class TypeParser<Processed, T extends Builder> extends AbstractGeometryFieldMapper.TypeParser<Builder> {
protected abstract Processed parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed);
public abstract static class TypeParser<T extends Builder> extends AbstractGeometryFieldMapper.TypeParser<Builder> {
protected abstract ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed);

@Override
@SuppressWarnings("rawtypes")
public T parse(String name, Map<String, Object> node, Map<String, Object> params, ParserContext parserContext) {
T builder = (T)(super.parse(name, node, params, parserContext));
parseField(builder, name, node, parserContext);
Expand All @@ -94,47 +103,47 @@ public T parse(String name, Map<String, Object> node, Map<String, Object> params
}

if (nullValue != null) {
builder.nullValue(parseNullValue(nullValue, (Boolean)builder.ignoreZValue().value(),
builder.setNullValue(parseNullValue(nullValue, (Boolean)builder.ignoreZValue().value(),
(Boolean)builder.ignoreMalformed().value()));
}

return builder;
}
}

final ParsedPoint nullValue;

public abstract static class AbstractPointGeometryFieldType<Parsed, Processed>
extends AbstractGeometryFieldType<Parsed, Processed> {
protected AbstractPointGeometryFieldType() {
super();
setHasDocValues(true);
setDimensions(2, Integer.BYTES);
protected AbstractPointGeometryFieldType(String name, boolean indexed, boolean hasDocValues, Map<String, String> meta) {
super(name, indexed, hasDocValues, meta);
}

protected AbstractPointGeometryFieldType(AbstractPointGeometryFieldType ref) {
super(ref);
}
}

protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType,
protected AbstractPointGeometryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
Settings indexSettings, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
Explicit<Boolean> ignoreZValue, CopyTo copyTo) {
super(simpleName, fieldType, defaultFieldType, indexSettings, ignoreMalformed, ignoreZValue, multiFields, copyTo);
Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo) {
super(simpleName, fieldType, mappedFieldType, indexSettings, ignoreMalformed, ignoreZValue, multiFields, copyTo);
this.nullValue = nullValue;
}

@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
AbstractPointGeometryFieldMapper gpfm = (AbstractPointGeometryFieldMapper)other;
if (gpfm.fieldType().nullValue() != null) {
this.fieldType().setNullValue(gpfm.fieldType().nullValue());
if (gpfm.nullValue != this.nullValue) {
conflicts.add("mapper [" + name() + "] has different [null_value] values");
}
}

@Override
@SuppressWarnings("rawtypes")
public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);
if (includeDefaults || fieldType().nullValue() != null) {
builder.field(Names.NULL_VALUE.getPreferredName(), fieldType().nullValue());
if (nullValue != null) {
builder.field(Names.NULL_VALUE.getPreferredName(), nullValue);
}
}

Expand Down Expand Up @@ -221,8 +230,8 @@ public XContentBuilder toXContent(List<P> points, XContentBuilder builder, Param
public List<P> fromXContent(XContentParser parser) throws IOException, ParseException {
P point = null;
ArrayList<P> points = null;
if (mapper.fieldType().nullValue() != null) {
point = (P)(mapper.fieldType().nullValue());
if (mapper.nullValue != null) {
point = (P)(mapper.nullValue);
if ((Boolean)(mapper.ignoreMalformed().value()) == false) {
point.validate(mapper.name());
} else {
Expand Down
Loading