Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove nested root from index pattern #54978

Merged
merged 8 commits into from
Jan 18, 2020
Merged
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 @@ -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