Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,15 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map
private static void parseObjectOrField(ParseContext context, Mapper mapper) throws IOException {
if (mapper instanceof ObjectMapper) {
parseObjectOrNested(context, (ObjectMapper) mapper);
} else {
FieldMapper fieldMapper = (FieldMapper)mapper;
} else if (mapper instanceof FieldMapper) {
FieldMapper fieldMapper = (FieldMapper) mapper;
Mapper update = fieldMapper.parse(context);
if (update != null) {
context.addDynamicMapper(update);
}
parseCopyFields(context, fieldMapper.copyTo().copyToFields());
} else {
throw new IllegalArgumentException("Cannot write to a field alias [" + mapper.name() + "].");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.index.mapper;

import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.support.XContentMapValues;

import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.Map;

public class FieldAliasMapper extends Mapper {
public static final String CONTENT_TYPE = "alias";

public static class Names {
public static final String PATH = "path";
}

private final String name;
private final String path;

public FieldAliasMapper(String simpleName,
String name,
String path) {
super(simpleName);
this.name = name;
this.path = path;
}

@Override
public String name() {
return name;
}

public String path() {
return path;
}

@Override
public Mapper merge(Mapper mergeWith) {
if (!(mergeWith instanceof FieldAliasMapper)) {
throw new IllegalArgumentException("Cannot merge a field alias [" + name() +
"] with a mapping that is not an alias [" + mergeWith.name() + "].");
}
return mergeWith;
}

@Override
public Mapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
return this;
}

@Override
public Iterator<Mapper> iterator() {
return Collections.emptyIterator();
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.startObject(simpleName())
.field("type", CONTENT_TYPE)
.field(Names.PATH, path)
.endObject();
}

public static class TypeParser implements Mapper.TypeParser {
@Override
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
throws MapperParsingException {
FieldAliasMapper.Builder builder = new FieldAliasMapper.Builder(name);
Object pathField = node.remove(Names.PATH);
String path = XContentMapValues.nodeStringValue(pathField, null);
if (path == null) {
throw new IllegalArgumentException("The [path] property must be specified.");
}
return builder.path(path);
Copy link

Choose a reason for hiding this comment

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

Im going to guess this is incomplete, but im going to guess here, that it wont be able to provide all the proper checking im performing on the path, eg:

  • does the path point to a real field.
  • does the path actually exist

etc.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Here's where I was thinking we would check if the alias was valid: https://github.com/jtibshirani/elasticsearch/pull/1/files#diff-c35d5ab4eb379e531c33305a916faceaR403. Other validation is already performed in MapperService/ FieldTypeLookup, so there is some precedent for this.

Copy link

Choose a reason for hiding this comment

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

@jtibshirani

image

Seem strange your comment says check here, and a few lines below it seems the state is updated ( is it ??) again.

Copy link

@mP1 mP1 May 30, 2018

Choose a reason for hiding this comment

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

Actually precent is really my concern, my concern is whether the entire state of mappings is complete by the stage you wish to perform your checks.

Is it possible for "updates" to happen "after" this spot, which will result in a mappings that have not all been verified ? Basically is it possible that the target of an alias, might point to one thing when 403 is executed but after that the target field is something else, and never gets checked.

}
}

public static class Builder extends Mapper.Builder<FieldAliasMapper.Builder, FieldAliasMapper> {
private String name;
private String path;

protected Builder(String name) {
super(name);
this.name = name;
}

public String name() {
return this.name;
}

public Builder path(String path) {
this.path = path;
return this;
}

public FieldAliasMapper build(BuilderContext context) {
String fullName = context.path().pathAsText(name);
return new FieldAliasMapper(name, fullName, path);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,40 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {

/** Full field name to field type */
final CopyOnWriteHashMap<String, MappedFieldType> fullNameToFieldType;
final CopyOnWriteHashMap<String, String> aliasToFullName;

/** Create a new empty instance. */
FieldTypeLookup() {
fullNameToFieldType = new CopyOnWriteHashMap<>();
aliasToFullName = new CopyOnWriteHashMap<>();
}

private FieldTypeLookup(CopyOnWriteHashMap<String, MappedFieldType> fullName) {
private FieldTypeLookup(CopyOnWriteHashMap<String, MappedFieldType> fullName,
CopyOnWriteHashMap<String, String> aliasToFullName) {
this.fullNameToFieldType = fullName;
this.aliasToFullName = aliasToFullName;
}

/**
* Return a new instance that contains the union of this instance and the field types
* from the provided fields. If a field already exists, the field type will be updated
* to use the new mappers field type.
*/

public FieldTypeLookup copyAndAddAll(String type, Collection<FieldMapper> fieldMappers) {
return copyAndAddAll(type, fieldMappers, new ArrayList<>());
}

public FieldTypeLookup copyAndAddAll(String type,
Collection<FieldMapper> fieldMappers,
Collection<FieldAliasMapper> fieldAliasMappers) {
Objects.requireNonNull(type, "type must not be null");
if (MapperService.DEFAULT_MAPPING.equals(type)) {
throw new IllegalArgumentException("Default mappings should not be added to the lookup");
}

CopyOnWriteHashMap<String, MappedFieldType> fullName = this.fullNameToFieldType;
CopyOnWriteHashMap<String, String> aliases = this.aliasToFullName;

for (FieldMapper fieldMapper : fieldMappers) {
MappedFieldType fieldType = fieldMapper.fieldType();
Expand All @@ -75,7 +87,14 @@ public FieldTypeLookup copyAndAddAll(String type, Collection<FieldMapper> fieldM
}
}
}
return new FieldTypeLookup(fullName);

for (FieldAliasMapper fieldAliasMapper : fieldAliasMappers) {
String aliasName = fieldAliasMapper.name();
String fieldName = fieldAliasMapper.path();
aliases = aliases.copyAndPut(aliasName, fieldName);
}

return new FieldTypeLookup(fullName, aliases);
}

/**
Expand All @@ -92,7 +111,10 @@ private void checkCompatibility(MappedFieldType existingFieldType, MappedFieldTy

/** Returns the field for the given field */
public MappedFieldType get(String field) {
return fullNameToFieldType.get(field);
String resolvedField = aliasToFullName.get(field);
return resolvedField == null
? fullNameToFieldType.get(field)
: fullNameToFieldType.get(resolvedField);
}

/**
Expand All @@ -105,6 +127,11 @@ public Collection<String> simpleMatchToFullName(String pattern) {
fields.add(fieldType.name());
}
}
for (String aliasName : aliasToFullName.keySet()) {
if (Regex.simpleMatch(pattern, aliasName)) {
fields.add(aliasName);
}
}
return fields;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import com.carrotsearch.hppc.ObjectHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;

import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
Expand Down Expand Up @@ -395,15 +394,17 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
// check basic sanity of the new mapping
List<ObjectMapper> objectMappers = new ArrayList<>();
List<FieldMapper> fieldMappers = new ArrayList<>();
List<FieldAliasMapper> fieldAliasMappers = new ArrayList<>();
Collections.addAll(fieldMappers, newMapper.mapping().metadataMappers);
MapperUtils.collect(newMapper.mapping().root(), objectMappers, fieldMappers);
MapperUtils.collect(newMapper.mapping().root(), objectMappers, fieldMappers, fieldAliasMappers);
checkFieldUniqueness(newMapper.type(), objectMappers, fieldMappers, fullPathObjectMappers, fieldTypes);
checkObjectsCompatibility(objectMappers, fullPathObjectMappers);
checkPartitionedIndexConstraints(newMapper);
// TODO: check aliases are valid here?

// update lookup data-structures
// this will in particular make sure that the merged fields are compatible with other types
fieldTypes = fieldTypes.copyAndAddAll(newMapper.type(), fieldMappers);
fieldTypes = fieldTypes.copyAndAddAll(newMapper.type(), fieldMappers, fieldAliasMappers);

for (ObjectMapper objectMapper : objectMappers) {
if (fullPathObjectMappers == this.fullPathObjectMappers) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,32 @@

package org.elasticsearch.index.mapper;

import java.util.ArrayList;
import java.util.Collection;

enum MapperUtils {
;

/** Split mapper and its descendants into object and field mappers. */
public static void collect(Mapper mapper, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
public static void collect(Mapper mapper, Collection<ObjectMapper> objectMappers,
Collection<FieldMapper> fieldMappers) {
collect(mapper, objectMappers, fieldMappers, new ArrayList<>());
}

public static void collect(Mapper mapper, Collection<ObjectMapper> objectMappers,
Collection<FieldMapper> fieldMappers,
Collection<FieldAliasMapper> fieldAliasMappers) {
if (mapper instanceof RootObjectMapper) {
// root mapper isn't really an object mapper
} else if (mapper instanceof ObjectMapper) {
objectMappers.add((ObjectMapper)mapper);
} else if (mapper instanceof FieldMapper) {
fieldMappers.add((FieldMapper)mapper);
} else if (mapper instanceof FieldAliasMapper) {
fieldAliasMappers.add((FieldAliasMapper) mapper);
}
for (Mapper child : mapper) {
collect(child, objectMappers, fieldMappers);
collect(child, objectMappers, fieldMappers, fieldAliasMappers);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.common.geo.ShapesAvailability;
import org.elasticsearch.common.inject.AbstractModule;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry;
import org.elasticsearch.index.mapper.FieldAliasMapper;
import org.elasticsearch.index.mapper.BinaryFieldMapper;
import org.elasticsearch.index.mapper.BooleanFieldMapper;
import org.elasticsearch.index.mapper.CompletionFieldMapper;
Expand Down Expand Up @@ -111,6 +112,8 @@ private Map<String, Mapper.TypeParser> getMappers(List<MapperPlugin> mapperPlugi
mappers.put(ObjectMapper.NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser());
mappers.put(CompletionFieldMapper.CONTENT_TYPE, new CompletionFieldMapper.TypeParser());
mappers.put(GeoPointFieldMapper.CONTENT_TYPE, new GeoPointFieldMapper.TypeParser());
mappers.put(FieldAliasMapper.CONTENT_TYPE, new FieldAliasMapper.TypeParser());

if (ShapesAvailability.JTS_AVAILABLE && ShapesAvailability.SPATIAL4J_AVAILABLE) {
mappers.put(GeoShapeFieldMapper.CONTENT_TYPE, new GeoShapeFieldMapper.TypeParser());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder;

/**
* Fetch phase of a search request, used to fetch the actual top matching documents to be returned to the client, identified
* after reducing all of the matches returned by the query phase
Expand Down Expand Up @@ -118,11 +116,12 @@ public void execute(SearchContext context) {
if (context.getObjectMapper(fieldName) != null) {
throw new IllegalArgumentException("field [" + fieldName + "] isn't a leaf field");
}
} else {
if (fieldNames == null) {
fieldNames = new HashSet<>();
}
fieldNames.add(fieldType.name());
}
if (fieldNames == null) {
fieldNames = new HashSet<>();
}
fieldNames.add(fieldName);
}
}
boolean loadSource = context.sourceRequested();
Expand Down
Loading