Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -20,6 +20,7 @@

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

public class IndexSortIT extends ESIntegTestCase {
private static final XContentBuilder TEST_MAPPING = createTestMapping();
Expand All @@ -28,6 +29,9 @@ private static XContentBuilder createTestMapping() {
try {
return jsonBuilder()
.startObject()
.startObject("runtime")
.startObject("alias").field("type", "alias").field("path", "numeric").endObject()
.endObject()
.startObject("properties")
.startObject("date")
.field("type", "date")
Expand Down Expand Up @@ -79,6 +83,18 @@ public void testIndexSort() {
flushAndRefresh();
ensureYellow();
assertSortedSegments("test", indexSort);

Exception e = expectThrows(Exception.class, () ->
prepareCreate("test_with_runtime_sort")
.setSettings(Settings.builder()
.put(indexSettings())
.put("index.number_of_shards", "1")
.put("index.number_of_replicas", "1")
.putList("index.sort.field", "alias")
)
.setMapping(TEST_MAPPING)
.get());
assertThat(e.getMessage(), equalTo("unknown concrete index sort field: [alias]"));
}

public void testInvalidIndexSort() {
Expand All @@ -91,7 +107,7 @@ public void testInvalidIndexSort() {
.setMapping(TEST_MAPPING)
.get()
);
assertThat(exc.getMessage(), containsString("unknown index sort field:[invalid_field]"));
assertThat(exc.getMessage(), containsString("unknown concrete index sort field: [invalid_field]"));

exc = expectThrows(IllegalArgumentException.class,
() -> prepareCreate("test")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public IndexService(
// we delay the actual creation of the sort order for this index because the mapping has not been merged yet.
// The sort order is validated right after the merge of the mapping later in the process.
this.indexSortSupplier = () -> indexSettings.getIndexSortConfig().buildIndexSort(
mapperService::fieldType,
mapperService::concreteFieldType,
(fieldType, searchLookup) -> indexFieldData.getForField(fieldType, indexFieldData.index().getName(), searchLookup)
);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public Sort buildIndexSort(Function<String, MappedFieldType> fieldTypeLookup,
FieldSortSpec sortSpec = sortSpecs[i];
final MappedFieldType ft = fieldTypeLookup.apply(sortSpec.field);
if (ft == null) {
throw new IllegalArgumentException("unknown index sort field:[" + sortSpec.field + "]");
throw new IllegalArgumentException("unknown concrete index sort field: [" + sortSpec.field + "]");
}
boolean reverse = sortSpec.order == null ? false : (sortSpec.order == SortOrder.DESC);
MultiValueMode mode = sortSpec.mode;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.index.mapper;

import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.List;
import java.util.function.Function;

public class AliasRuntimeField implements RuntimeFieldType {

public static final String CONTENT_TYPE = "alias";

private static class Builder extends RuntimeFieldType.Builder {

final FieldMapper.Parameter<String> path = FieldMapper.Parameter.stringParam(
"path",
true,
RuntimeFieldType.initializerNotSupported(),
null
).setValidator(
s -> {
if (s == null) {
throw new MapperParsingException("Missing required parameter [path]");
}
}
);

protected Builder(String name) {
super(name);
}

@Override
protected List<FieldMapper.Parameter<?>> getParameters() {
return List.of(path);
}

@Override
protected RuntimeFieldType buildFieldType() {
return new AliasRuntimeField(name, path.get());
}
}

public static final RuntimeFieldType.Parser PARSER = new RuntimeFieldType.Parser((n, c) -> new Builder(n));

private final String name;
private final String path;

public AliasRuntimeField(String name, String path) {
this.name = name;
this.path = path;
}

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

@Override
public MappedFieldType asMappedFieldType(Function<String, MappedFieldType> lookup) {
MappedFieldType ft = lookup.apply(path);
if (ft == null) {
throw new IllegalStateException("Cannot resolve alias [" + name + "]: path [" + path + "] does not exist");
Copy link
Contributor

Choose a reason for hiding this comment

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

Small comment, instead of saying 'does not exist' we could say something like 'the field [path] either does not exist or is a runtime field'?

}
return ft;
}

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

@Override
public String typeName() {
return CONTENT_TYPE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
Expand All @@ -24,6 +25,7 @@
*/
final class FieldTypeLookup {
private final Map<String, MappedFieldType> fullNameToFieldType = new HashMap<>();
private final Map<String, RuntimeFieldType> runtimeFields = new HashMap<>();

/**
* A map from field name to all fields whose content has been copied into it
Expand Down Expand Up @@ -71,7 +73,7 @@ final class FieldTypeLookup {

for (RuntimeFieldType runtimeFieldType : runtimeFieldTypes) {
//this will override concrete fields with runtime fields that have the same name
Copy link
Member

Choose a reason for hiding this comment

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

did you remove this comment on purpose?

fullNameToFieldType.put(runtimeFieldType.name(), runtimeFieldType);
runtimeFields.put(runtimeFieldType.name(), runtimeFieldType);
}

this.dynamicKeyLookup = new DynamicKeyFieldTypeLookup(dynamicKeyMappers, aliasToConcreteName);
Expand All @@ -81,6 +83,33 @@ final class FieldTypeLookup {
* Returns the mapped field type for the given field name.
*/
MappedFieldType get(String field) {
GuardedFieldTypeLookup guardedLookup = new GuardedFieldTypeLookup();
return guardedLookup.get(field);
}

private class GuardedFieldTypeLookup {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's really valuable to support aliases to other aliases. The same goes for aliases to other runtime fields. I can't see a strong use case for this -- the user can already easily rename fields in the runtime section, or just add the definition twice if they need both fields present. Aliases are most helpful because you can't update a 'standard' field without reindexing.

Do you have some use cases in mind related to runtime fields? I'm curious because supporting this adds complexity here and in SearchExecutionContext to prevent loops. I wonder if it'd be the right trade-off to require the path to point to a standard field (defined under properties).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think aliases to other aliases is useful, but possibly only at query time. I'll try moving things around a bit to only support query-time lookups, and that may simplify other stuff as well.


Set<String> fieldPath = new LinkedHashSet<>();

MappedFieldType get(String field) {
if (fieldPath.contains(field)) {
throw new IllegalStateException("Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field);
}
fieldPath.add(field);
RuntimeFieldType runtimeFieldType = FieldTypeLookup.this.runtimeFields.get(field);
if (runtimeFieldType != null) {
return runtimeFieldType.asMappedFieldType(this::get);
}
return getConcrete(field);
}
}

/**
* Returns the mapped field type for the given field, excluding runtime fields
* @param field the field name
* @return a mapped field type, or null if the field does not exist
*/
MappedFieldType getConcrete(String field) {
MappedFieldType fieldType = fullNameToFieldType.get(field);
if (fieldType != null) {
return fieldType;
Expand All @@ -100,6 +129,11 @@ Set<String> simpleMatchToFullName(String pattern) {
return Collections.singleton(pattern);
}
Set<String> fields = new HashSet<>();
for (String field : runtimeFields.keySet()) {
if (Regex.simpleMatch(pattern, field)) {
fields.add(field);
}
}
for (String field : fullNameToFieldType.keySet()) {
if (Regex.simpleMatch(pattern, field)) {
fields.add(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ public MappedFieldType fieldType(String fullName) {
return mappingLookup().fieldTypes().get(fullName);
}

public MappedFieldType concreteFieldType(String field) {
return mappingLookup().fieldTypes().getConcrete(field);
}

/**
* Returns all the fields that match the given pattern. If the pattern is prefixed with a type
* then the fields will be returned with a type prefix.
Expand Down
Loading