Add multi field info to the IndexPattern #33681

Bargs · 2019-03-21T23:04:22Z

Summary

Fixes #31921
Related to #7419

Adds two fields to the IndexPattern Field:

parent - the name of the field this field is a child of
subType - The type of child this field is. Currently the only valid value is multi but we could expand this to include aliases, object children, and nested children.

The thinking behind implementing these two new properties instead of a simple isMultiField flag is that it should be generic enough to describe other sorts of parent -> child relationships between fields.

I need to update some tests and I want to check and make sure nothing special needs to be done for rollups, but beyond that I believe the implementation is complete.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~- [ ] Documentation was added for features that require explanation or tutorials~~

Unit or functional tests were updated or added to match the most common scenarios
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2019-03-21T23:04:24Z

Pinging @elastic/kibana-app

elasticmachine · 2019-03-21T23:53:07Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

nreese

thanks for adding this information toField. I was able to filter out multi fields in the Maps application and it worked as expected.

Should this PR also update the index-pattern saved objects for the sample data sets?

lgtm
code review, tested in chrome.

nreese · 2019-03-22T19:06:20Z

src/legacy/server/index_patterns/service/lib/field_capabilities/field_caps_response.js

+    const parentFieldName = field.name.split('.').slice(0, -1).join('.');
+    const parentFieldCaps = kibanaFormattedCaps.find(caps => caps.name === parentFieldName);
+
+    if (!['object', 'nested'].includes(parentFieldCaps.type)) {


Should you check that parentFieldCaps is not undefined since find could return undefined if parentFieldName can not be found?

You should probably also check if the type is 'join' as well: https://www.elastic.co/guide/en/elasticsearch/reference/current/parent-join.html

Added check for undefined.

I wasn't sure how join would be reported in field_caps so I tested it out. Looks like we don't need to worry about it. It's a bit different because the relationship is between documents, not fields, so the relationship is not represented with a dot in the field name. Here's a sample response with the extraneous meta fields removed:

{ "fields" : { "my_join_field#question" : { "parent" : { "type" : "parent", "searchable" : true, "aggregatable" : true } }, "_parent_join" : { "parent_join" : { "type" : "parent_join", "searchable" : false, "aggregatable" : false } }, "my_join_field" : { "join" : { "type" : "join", "searchable" : true, "aggregatable" : true } }, "text" : { "text" : { "type" : "text", "searchable" : true, "aggregatable" : false } }, "text.keyword" : { "keyword" : { "type" : "keyword", "searchable" : true, "aggregatable" : true } }, } }

lukasolson

Just after a cursory glance I think there are probably some other files that should be updated with these new properties (do a global search for readFromDocValues).

Also, did we want to include the actual field type as well? (e.g. whether it's text or keyword, etc.)

lukasolson · 2019-03-26T23:15:57Z

src/legacy/server/index_patterns/service/lib/field_capabilities/field_caps_response.js

+
+    if (!['object', 'nested'].includes(parentFieldCaps.type)) {
+      field.parent = parentFieldName;
+      field.subType = 'multi';


Why not just have a isMulti boolean instead of adding another string property subType? What else could subType be?

Do we even need the multi designation? Can we just assume if parent is not undefined, it's multi?

In the future it could be nested, object, or join. I see this being used to describe any parent/child relationship between fields. I thought about just doing an isMulti flag but then we'd need to add isNested, isObject and isJoin in the future.

Instead of having a separate parent field and a flag we could also just have something like multiParent, nestedParent, objectParent, joinParent. We'd still have to add a new one for each new relationship type, but then they're not coupled together.

I dunno, what do you think?

I've made all the updates I can without making a decision on this. Once we've agreed on a format I'll go through and update all the other places mentioned that need these properties.

Bargs · 2019-03-28T15:08:23Z

Also, did we want to include the actual field type as well? (e.g. whether it's text or keyword, etc.)

I think this is an orthogonal concern. Having more detailed type info would be useful for all fields, not just subfields.

elasticmachine · 2019-03-28T15:49:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lukasolson

LGTM

elasticmachine · 2019-03-28T22:13:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-02T16:49:01Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-02T17:34:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

Adds two fields to the IndexPattern Field: * parent - the name of the field this field is a child of * subType - The type of child this field is. Currently the only valid value is multi but we could expand this to include aliases, object children, and nested children. The thinking behind implementing these two new properties instead of a simple isMultiField flag is that it should be generic enough to describe other sorts of parent -> child relationships between fields.

Bargs · 2019-04-03T16:05:33Z

@nreese @lukasolson embarrassingly, one thing I didn't think about until now is that old index patterns will not have this additional info until the fields are manually refreshed. I've attempted to hack my own auto field refresh in situations like this in the past and it turned out poorly with lots of edge cases.

As I understand it, our formal migration API cannot currently make http requests or access services because it happens early in the start up process. @elastic/kibana-platform am I right about this? Is there any way I could refresh the index pattern fields during a migration between minor versions (7.0 -> 7.1)?

nreese · 2019-04-03T17:03:29Z

Is there any way I could refresh the index pattern fields during a migration between minor versions (7.0 -> 7.1)

I am not sure users will appreciate having their index-pattern saved objects auto-refreshed when upgrading without some kind of explicit opt-in. Updating the index-pattern can introduce new fields and such if the index has changed. I think expecting the user to update their index-patterns on their own is ok.

Bargs · 2019-04-03T21:59:37Z

I think expecting the user to update their index-patterns on their own is ok.

Ok, that's good input. One of my worries was that in maps you would not want the multi fields showing up until the user manually refreshes.

Bargs added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.2.0 labels Mar 21, 2019

Bargs requested review from nreese, timroes and lukasolson March 21, 2019 23:04

nreese approved these changes Mar 22, 2019

View reviewed changes

lukasolson reviewed Mar 22, 2019

View reviewed changes

lukasolson reviewed Mar 26, 2019

View reviewed changes

Bargs added the release_note:skip Skip the PR/issue when compiling release notes label Mar 27, 2019

Bargs force-pushed the addMultiInfo branch from 5925787 to 3787731 Compare March 28, 2019 15:06

lukeelmers added Feature:Data Views Data Views code and UI - index patterns before 8.0 :AppArch labels Mar 28, 2019

lukasolson approved these changes Mar 28, 2019

View reviewed changes

Bargs added 8 commits April 2, 2019 12:10

Add multi info to the _fields_for_x API end points

d886043

Add multi info to the IndexPattern object

264a753

Add multi info to the Field interface

a52b58c

Check for undefined

324e0f7

Include forgotten test suite and updates its tests with the multi info

2222f91

Update fields with multi info where necessary

3ab3185

Update fixtures with multi field info

bd7376c

Update es archiver fixtures with multi info

9cbd07e

Bargs force-pushed the addMultiInfo branch from b18edd3 to 9cbd07e Compare April 2, 2019 16:12

update field_caps_response test

e50ecc1

Bargs merged commit d8916e3 into elastic:master Apr 2, 2019

Bargs mentioned this pull request Apr 22, 2019

Add esTypes property to index pattern field #35251

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi field info to the IndexPattern #33681

Add multi field info to the IndexPattern #33681

Bargs commented Mar 21, 2019 •

edited

Loading

elasticmachine commented Mar 21, 2019

elasticmachine commented Mar 21, 2019

nreese left a comment

nreese Mar 22, 2019

lukasolson Mar 26, 2019

Bargs Mar 28, 2019

lukasolson left a comment

lukasolson Mar 26, 2019

lukasolson Mar 26, 2019

Bargs Mar 27, 2019

Bargs Mar 28, 2019

Bargs commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

lukasolson left a comment

elasticmachine commented Mar 28, 2019

elasticmachine commented Apr 2, 2019

elasticmachine commented Apr 2, 2019

Bargs commented Apr 3, 2019

nreese commented Apr 3, 2019

Bargs commented Apr 3, 2019

Add multi field info to the IndexPattern #33681

Add multi field info to the IndexPattern #33681

Conversation

Bargs commented Mar 21, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 21, 2019

elasticmachine commented Mar 21, 2019

💔 Build Failed

nreese left a comment

Choose a reason for hiding this comment

nreese Mar 22, 2019

Choose a reason for hiding this comment

lukasolson Mar 26, 2019

Choose a reason for hiding this comment

Bargs Mar 28, 2019

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

lukasolson Mar 26, 2019

Choose a reason for hiding this comment

lukasolson Mar 26, 2019

Choose a reason for hiding this comment

Bargs Mar 27, 2019

Choose a reason for hiding this comment

Bargs Mar 28, 2019

Choose a reason for hiding this comment

Bargs commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

💚 Build Succeeded

lukasolson left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 28, 2019

💚 Build Succeeded

elasticmachine commented Apr 2, 2019

💚 Build Succeeded

elasticmachine commented Apr 2, 2019

💚 Build Succeeded

Bargs commented Apr 3, 2019

nreese commented Apr 3, 2019

Bargs commented Apr 3, 2019

Bargs commented Mar 21, 2019 •

edited

Loading