Skip to content

Commit

Permalink
Remove nested root from index pattern (#54978) (#55267)
Browse files Browse the repository at this point in the history
* Revert "Add label and icon to nested fields in the doc table (#54199)"

This reverts commit f77b362

* Apply label and icon to nested fields in the doc table

* Add nested type to field_icon

* Improve nested test and add comment

* Fix tests

* Always pass the field type

Co-authored-by: Matt Bargar <mbargar@gmail.com>
  • Loading branch information
Tim Roes and Bargs committed Jan 18, 2020
1 parent f7f28b7 commit 7e7b18d
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,50 +26,48 @@ import { IndexPattern, indexPatterns } from '../../../kibana_services';

jest.mock('ui/new_platform');

// @ts-ignore
const indexPattern = {
fields: {
getByName: (name: string) => {
const fields: { [name: string]: {} } = {
_index: {
name: '_index',
type: 'string',
scripted: false,
filterable: true,
},
message: {
name: 'message',
type: 'string',
scripted: false,
filterable: false,
},
extension: {
name: 'extension',
type: 'string',
scripted: false,
filterable: true,
},
bytes: {
name: 'bytes',
type: 'number',
scripted: false,
filterable: true,
},
scripted: {
name: 'scripted',
type: 'number',
scripted: true,
filterable: false,
},
};
return fields[name];
fields: [
{
name: '_index',
type: 'string',
scripted: false,
filterable: true,
},
{
name: 'message',
type: 'string',
scripted: false,
filterable: false,
},
{
name: 'extension',
type: 'string',
scripted: false,
filterable: true,
},
},
{
name: 'bytes',
type: 'number',
scripted: false,
filterable: true,
},
{
name: 'scripted',
type: 'number',
scripted: true,
filterable: false,
},
],
metaFields: ['_index', '_score'],
flattenHit: undefined,
formatHit: jest.fn(hit => hit._source),
} as IndexPattern;

indexPattern.fields.getByName = (name: string) => {
return indexPattern.fields.find(field => field.name === name);
};

indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields);

describe('DocViewTable at Discover', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import React, { useState } from 'react';
import { escapeRegExp } from 'lodash';
import { DocViewTableRow } from './table_row';
import { arrayContainsObjects, trimAngularSpan } from './table_helper';
import { DocViewRenderProps } from '../../doc_views/doc_views_types';
Expand Down Expand Up @@ -68,11 +69,57 @@ export function DocViewTable({
const displayNoMappingWarning =
!mapping(field) && !displayUnderscoreWarning && !isArrayOfObjects;

// Discover doesn't flatten arrays of objects, so for documents with an `object` or `nested` field that
// contains an array, Discover will only detect the top level root field. We want to detect when those
// root fields are `nested` so that we can display the proper icon and label. However, those root
// `nested` fields are not a part of the index pattern. Their children are though, and contain nested path
// info. So to detect nested fields we look through the index pattern for nested children
// whose path begins with the current field. There are edge cases where
// this could incorrectly identify a plain `object` field as `nested`. Say we had the following document
// where `foo` is a plain object field and `bar` is a nested field.
// {
// "foo": [
// {
// "bar": [
// {
// "baz": "qux"
// }
// ]
// },
// {
// "bar": [
// {
// "baz": "qux"
// }
// ]
// }
// ]
// }
//
// The following code will search for `foo`, find it at the beginning of the path to the nested child field
// `foo.bar.baz` and incorrectly mark `foo` as nested. Any time we're searching for the name of a plain object
// field that happens to match a segment of a nested path, we'll get a false positive.
// We're aware of this issue and we'll have to live with
// it in the short term. The long term fix will be to add info about the `nested` and `object` root fields
// to the index pattern, but that has its own complications which you can read more about in the following
// issue: https://github.com/elastic/kibana/issues/54957
const isNestedField =
!indexPattern.fields.find(patternField => patternField.name === field) &&
!!indexPattern.fields.find(patternField => {
// We only want to match a full path segment
const nestedRootRegex = new RegExp(escapeRegExp(field) + '(\\.|$)');
return nestedRootRegex.test(patternField.subType?.nested?.path ?? '');
});
const fieldType = isNestedField
? 'nested'
: indexPattern.fields.find(patternField => patternField.name === field)?.type;

return (
<DocViewTableRow
key={field}
field={field}
fieldMapping={mapping(field)}
fieldType={fieldType}
displayUnderscoreWarning={displayUnderscoreWarning}
displayNoMappingWarning={displayNoMappingWarning}
isCollapsed={isCollapsed}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { DocViewTableRowIconUnderscore } from './table_row_icon_underscore';
export interface Props {
field: string;
fieldMapping?: FieldMapping;
fieldType?: string;
displayNoMappingWarning: boolean;
displayUnderscoreWarning: boolean;
isCollapsible: boolean;
Expand All @@ -46,6 +47,7 @@ export interface Props {
export function DocViewTableRow({
field,
fieldMapping,
fieldType,
displayNoMappingWarning,
displayUnderscoreWarning,
isCollapsible,
Expand Down Expand Up @@ -85,7 +87,7 @@ export function DocViewTableRow({
</td>
)}
<td className="kbnDocViewer__field">
<FieldName field={fieldMapping} fieldName={field} />
<FieldName field={fieldMapping} fieldName={field} fieldType={fieldType} />
</td>
<td>
{isCollapsible && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('index_patterns/field_capabilities/field_caps_response', () => {
describe('conflicts', () => {
it('returns a field for each in response, no filtering', () => {
const fields = readFieldCapsResponse(esResponse);
expect(fields).toHaveLength(25);
expect(fields).toHaveLength(24);
});

it(
Expand Down Expand Up @@ -68,8 +68,8 @@ describe('index_patterns/field_capabilities/field_caps_response', () => {
sandbox.spy(shouldReadFieldFromDocValuesNS, 'shouldReadFieldFromDocValues');
const fields = readFieldCapsResponse(esResponse);
const conflictCount = fields.filter(f => f.type === 'conflict').length;
// +1 is for the object field which is filtered out of the final return value from readFieldCapsResponse
sinon.assert.callCount(shouldReadFieldFromDocValues, fields.length - conflictCount + 1);
// +2 is for the object and nested fields which get filtered out of the final return value from readFieldCapsResponse
sinon.assert.callCount(shouldReadFieldFromDocValues, fields.length - conflictCount + 2);
});

it('converts es types to kibana types', () => {
Expand Down Expand Up @@ -159,12 +159,10 @@ describe('index_patterns/field_capabilities/field_caps_response', () => {
});
});

it('returns the nested parent as not searchable or aggregatable', () => {
it('does not include the field actually mapped as nested itself', () => {
const fields = readFieldCapsResponse(esResponse);
const child = fields.find(f => f.name === 'nested_object_parent');
expect(child.type).toBe('nested');
expect(child.aggregatable).toBe(false);
expect(child.searchable).toBe(false);
expect(child).toBeUndefined();
});

it('should not confuse object children for multi or nested field children', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,6 @@ export function readFieldCapsResponse(fieldCapsResponse: FieldCapsResponse): Fie
});

return kibanaFormattedCaps.filter(field => {
return !['object'].includes(field.type);
return !['object', 'nested'].includes(field.type);
});
}
4 changes: 2 additions & 2 deletions src/plugins/kibana_react/public/field_icon/field_icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ interface FieldIconProps {
| 'number'
| '_source'
| 'string'
| 'nested'
| string;
| string
| 'nested';
label?: string;
size?: IconSize;
useColor?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ export default function({ getService }) {
name: 'foo',
readFromDocValues: true,
},
{
aggregatable: false,
esTypes: ['nested'],
name: 'nestedField',
readFromDocValues: false,
searchable: false,
type: 'nested',
},
{
aggregatable: false,
esTypes: ['keyword'],
Expand Down Expand Up @@ -161,14 +153,6 @@ export default function({ getService }) {
name: 'foo',
readFromDocValues: true,
},
{
aggregatable: false,
esTypes: ['nested'],
name: 'nestedField',
readFromDocValues: false,
searchable: false,
type: 'nested',
},
{
aggregatable: false,
esTypes: ['keyword'],
Expand Down

0 comments on commit 7e7b18d

Please sign in to comment.