Add aliases as runtime fields #70065

romseygeek · 2021-03-08T11:28:40Z

Alias fields allow you to correct errors in mappings or to unify mappings from
multiple sources by adding a redirection from one field name to another. However,
as currently implemented they have the drawbacks of a concrete field: they cannot
be deleted, and they apply to all clients. They can also only refer directly to
concrete fields and cannot link to other aliases.

This commit adds a new concept of alias fields as runtime fields. Like any other runtime
field they can be defined in the runtime section of an index's mappings, or in a search
request; they can be updated or deleted without restriction; and they can refer to
other runtime fields as well as to concrete fields, with some loop detection to prevent
stack overflows. To support this functionality, RuntimeField's asMappedFieldType()
method is extended to take a field type lookup function.

elasticmachine · 2021-03-08T11:28:43Z

Pinging @elastic/es-search (Team:Search)

server/src/main/java/org/elasticsearch/index/IndexService.java

server/src/main/java/org/elasticsearch/index/IndexSortConfig.java

server/src/main/java/org/elasticsearch/index/mapper/RuntimeFieldType.java

server/src/test/java/org/elasticsearch/index/IndexSortSettingsTests.java

server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java

romseygeek · 2021-03-08T11:38:27Z

I got inspired over the weekend after the search meeting on Friday and came up with this, what do people think?

javanna

I left some initial comments. I also have a couple of meta points:

this introduces a bit of complexity on the runtime fields existing structure, and aliases can already be defined on the runtime section through a simple script, more or less? If we do implement a specialized alias field on the runtime section, I wonder if these's a way to make it fit in the existing design without structural changes, similar to what we did for script-less runtime fields.
what plan could we make to remove the existing alias field mapper if any?

server/src/main/java/org/elasticsearch/index/mapper/AliasRuntimeField.java

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

server/src/main/java/org/elasticsearch/index/IndexSortConfig.java

server/src/main/java/org/elasticsearch/index/mapper/RuntimeFieldType.java

server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java

romseygeek · 2021-03-08T15:27:59Z

aliases can already be defined on the runtime section through a simple script, more or less?

Well, sort of... but only for fields with types that we've implemented runtime script fields for (so not text fields, for example). And even for those the script isn't that simple, because you need to take into account empty and/or multiple values:

"runtime": {
  "not-quite-an-alias": {
    "type": "long",
    "script" : "for (long v : doc['field']) { emit(v); }"
  }
}

vs

"runtime": {
  "alias" : {
    "type": "alias",
    "path": "field"
  }
}

what plan could we make to remove the existing alias field mapper if any?

I think internally it's pretty easy to make FieldAliasMapper return a runtime alias and deal with that in the MappingLookup constructor, which lets us then remove all the special-casing elsewhere. Then we add a deprecation warning for any mappings that use an alias field in properties, refuse to create them in 8.x and remove entirely in 9.x

Merge remote-tracking branch 'origin/master' into runtime/aliases

jtibshirani · 2021-03-29T17:11:30Z

I'm excited about this effort! I understand it's on pause now, so am holding off on a detailed review. For context, here are two issues I think this change would address:

Support deleting aliases from mappings (Support to remove aliases from index mappings #36418)
Field alias updates are not reflected in percolator queries (Field alias updates are not reflected in percolator queries. #37212)

romseygeek · 2021-04-26T09:43:37Z

I've updated this to the latest master - the changes that @javanna made in #70695 incorporate a lot of what I had initially included here, so this ends up being a much smaller change.

I have not made any attempt to change the existing FieldAliasMapper to use this functionality as well. I don't think it will be tricky, but it will require another look at dynamic field type lookups as this alias implementation does not support flattened fields.

romseygeek · 2021-04-26T11:00:23Z

server/src/test/java/org/elasticsearch/index/mapper/RuntimeAliasTests.java

+        assertEquals("Loop in field resolution detected: alias-loop1->alias-loop2->alias-loop1", e.getMessage());
+    }
+
+    /*


This commented-out test illustrates the problem with dynamic lookups that would be fixed by #72024

javanna · 2021-05-11T12:49:38Z

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java

-            MappedFieldType runtimeFieldType = runtimeField.asMappedFieldType();
-            //this will override concrete fields with runtime fields that have the same name
-            fullNameToFieldType.put(runtimeFieldType.name(), runtimeFieldType);
+            this.runtimeFields.put(runtimeField.name(), runtimeField);


I am a bit confused about why we need to introduce the distinction between ordinary fields and runtime fields in here. An alias can alias any field, what difference does it make if it's a runtime field or not?

It's because asMappedFieldType() takes a MappingLookup now, so we can't resolve this at construction time.

I see, maybe another reason to try and change asMappedFieldType then.

romseygeek · 2021-05-12T14:54:33Z

I've simplified the lookup logic considerably:

runtime MappedFieldTypes are resolved when FieldTypeLookup or SearchExecutionContext are built
aliases defined in the runtime section can only refer to fields in the properties section
aliases defined in a query can refer to anything in runtime or in properties, but can't refer to other fields defined in the query

This removes the need for loop detection. It has the downside that you can't alias to a path within a dynamic field any more, but that wasn't previously supported so we aren't losing much.

jtibshirani

The approach looks simpler now, it seems like a good trade-off to only allow aliases to refer to concrete fields. My last high-level comment: It'd be nice to add some light docs in this PR that explain the behavior and restrictions.

jtibshirani · 2021-05-12T17:07:56Z

server/src/main/java/org/elasticsearch/index/mapper/RuntimeField.java

     * @return the {@link MappedFieldType} backing this runtime field
     */
-    MappedFieldType asMappedFieldType();
+    MappedFieldType asMappedFieldType(Function<String, MappedFieldType> lookup);


I am not super happy about this method. I added it as I thought we needed it for the object runtime field, but it turns out we won't use it...

After thinking about this more, having some method like this makes sense to me in general. It seems like RuntimeField is a counterpart to FieldMapper in that it contains a mapping definition and is able to produce a MappedFieldType. I wonder if we could rename the method to fieldType to better fit with FieldMapper?

As for the function argument, so far I don't see a cleaner approach. To me it's not too strange that runtime fields have access to the concrete field types here. @romseygeek pointed out that we've brainstormed new types of runtime fields that refer to concrete fields -- such as one that combines the contents of multiple text fields like a virtual copy_to.

jtibshirani · 2021-05-12T17:17:58Z

server/src/main/java/org/elasticsearch/index/mapper/AliasRuntimeField.java

+    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");


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'?

romseygeek · 2021-05-13T08:25:24Z

For docs, I'd like to ask @lockewritesdocs for his opinion on where to add them. The runtime.asciidoc page currently assumes that all runtime fields are defined with a script, and I think will need some reworking so that docs for this new runtime type fit in.

lockewritesdocs · 2021-05-13T17:13:27Z

For docs, I'd like to ask @lockewritesdocs for his opinion on where to add them. The runtime.asciidoc page currently assumes that all runtime fields are defined with a script, and I think will need some reworking so that docs for this new runtime type fit in.

@romseygeek, I think that we should introduce aliases for runtime fields through a new section in both Mapping a runtime field and Defining runtime fields in a search request, since aliases for runtime fields are supported in both instances. If you can provide some examples and explanatory text, I can create a PR to update the docs in those sections.

javanna

I left a couple of comments. I think this is close, we only need to iron out a couple of design details. One thing that I am not sold on is supporting aliases to other runtime fields only in the search request. I think if you can only alias a concrete field, the behaviour should be consistent no matter where the alias is defined. I can imagine that users may want us to support aliasing a runtime field, but runtime fields are editable, and defined through a script, so the alias functionality really applies to concrete fields that cannot be changed.

javanna · 2021-05-14T10:07:43Z

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java


        for (RuntimeField runtimeField : runtimeFields) {
-            MappedFieldType runtimeFieldType = runtimeField.asMappedFieldType();
-            //this will override concrete fields with runtime fields that have the same name


did you remove this comment on purpose?

javanna · 2021-05-14T10:11:04Z

server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java

+        if (runtimeMappings.containsKey(name)) {
+            return runtimeMappings.get(name);
+        }
+        return mappingLookup.getFieldType(name);


do we gain anything with this change?

javanna · 2021-05-14T13:42:14Z

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java

-            MappedFieldType runtimeFieldType = runtimeField.asMappedFieldType();
-            //this will override concrete fields with runtime fields that have the same name
-            fullNameToFieldType.put(runtimeFieldType.name(), runtimeFieldType);
+            MappedFieldType resolved = runtimeField.asMappedFieldType(fullNameToFieldType::get);


thinking out loud: if we use asMappedFieldType only for aliases, given that all existing runtime fields return this, should we consider adding the notion of alias to RuntimeField through a method that returns a String and return by default the same as the field name, and when it differs look it up directly in FieldTypeLookup? Maybe it is cryptic, not too sure, but it would make it more specific to aliases (which is both good and bad I guess), would remove the need to delegate to the runtime field to look up the aliased field type. I do wonder if it should be the responsibility of the field type to look up the aliases field among all fields. We may or may not need something more generic at some point in the future but we can always refactor things later when needed?

one more thing: fullNameToFieldType does not include the previously resolved alias field mapper, so we are sure that an alias runtime field never points to an alias field defined under properties?

javanna · 2021-05-14T13:49:08Z

server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java

-            if (fieldType instanceof DynamicFieldType) {
-                dynamicFieldTypes.put(aliasName, (DynamicFieldType) fieldType);
+            RuntimeField aliasField = new AliasRuntimeField(aliasName, path);
+            MappedFieldType resolved = aliasField.asMappedFieldType(fullNameToFieldType::get);


This means that the internal representation of an alias field as it existed so far is the same as the runtime alias field, but this has no effect on how it gets serialized and sent out throughout the nodes and stored in the cluster state, correct?

elasticsearchmachine · 2022-02-02T20:48:47Z

Hi @romseygeek, I've created a changelog YAML for you.

javanna · 2022-06-23T12:57:30Z

Heya, I opened an issue for this so we track the need for it: see #87969 . I assume that this PR is outdated and can't be brought up-to-date, hence closing.

Add aliases as runtime fields

83d921d

romseygeek added >feature :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.13.0 labels Mar 8, 2021

romseygeek requested review from javanna and jtibshirani March 8, 2021 11:28

romseygeek self-assigned this Mar 8, 2021

elasticmachine added the Team:Search Meta label for search team label Mar 8, 2021

romseygeek commented Mar 8, 2021

View reviewed changes

javanna reviewed Mar 8, 2021

View reviewed changes

romseygeek added 2 commits March 9, 2021 11:04

Merge remote-tracking branch 'origin/master' into runtime/aliases

74b4349

Merge remote-tracking branch 'origin/master' into runtime/aliases

cdeb51a

romseygeek mentioned this pull request Mar 15, 2021

Add calculated numeric fields #69531

Merged

romseygeek added 3 commits March 25, 2021 13:43

Slightly broken in IndexSortConfig

b9e8e3a

Merge remote-tracking branch 'origin/master' into runtime/aliases

Merge remote-tracking branch 'origin/master' into runtime/aliases

33dbf9c

Merge remote-tracking branch 'origin/master' into runtime/aliases

c7c0db4

pugnascotia added v7.14.0 and removed v7.13.0 labels Apr 21, 2021

romseygeek mentioned this pull request Apr 21, 2021

Merge dynamic field type lookup into FieldTypeLookup #72024

Merged

Merge remote-tracking branch 'origin/master' into runtime/aliases

c49ed9c

romseygeek requested a review from javanna April 26, 2021 09:44

Add commented-out test showing problem with dynamic alias lookup

88bed88

romseygeek commented Apr 26, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into runtime/aliases

fdd27a4

javanna reviewed May 11, 2021

View reviewed changes

romseygeek added 2 commits May 12, 2021 13:35

Merge remote-tracking branch 'origin/master' into runtime/aliases

3389a2d

Simplify alias lookups

175f442

jtibshirani reviewed May 12, 2021

View reviewed changes

test fixes; reword error messages

9151775

romseygeek added 3 commits May 13, 2021 12:00

Merge remote-tracking branch 'origin/master' into runtime/aliases

94fc104

hmm

683d1be

cmon

0b4ca87

javanna reviewed May 14, 2021

View reviewed changes

mark-vieira added v7.15.0 and removed v7.14.0 labels Jun 30, 2021

mark-vieira added v7.16.0 and removed v7.15.0 labels Aug 19, 2021

danhermann added v8.1.0 and removed v7.16.0 labels Oct 27, 2021

arteam removed the v8.0.0 label Jan 12, 2022

mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022

Update docs/changelog/70065.yaml

38ce282

salvatore-campagna added v8.3.0 and removed v8.2.0 labels Mar 30, 2022

craigtaverner added v8.4.0 and removed v8.3.0 labels May 25, 2022

javanna removed the v8.4.0 label Jun 23, 2022

javanna closed this Jun 23, 2022

Add aliases as runtime fields #70065

Add aliases as runtime fields #70065

Uh oh!

Conversation

romseygeek commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Mar 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romseygeek commented Mar 8, 2021

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romseygeek commented Mar 8, 2021

Uh oh!

jtibshirani commented Mar 29, 2021

Uh oh!

romseygeek commented Apr 26, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romseygeek commented May 12, 2021

Uh oh!

jtibshirani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romseygeek commented May 13, 2021

Uh oh!

lockewritesdocs commented May 13, 2021

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Feb 2, 2022

Uh oh!

javanna commented Jun 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

romseygeek commented Mar 8, 2021 •

edited

Loading