Skip to content

Commit

Permalink
[AggConfigs] Consider root level filters buckets correctly when build…
Browse files Browse the repository at this point in the history
…ing other terms bucket (#165656)

## Summary

Fixes #165487 

This PR fixes the `Other` bucket problem when the filter is used at root
level with a nested `terms` agg.
This is visible when using the `Split metric by` in Lens in a table
visualization.

Used the `sum` to easily compare the results:

<img width="619" alt="Screenshot 2023-09-05 at 11 12 46"
src="https://github.com/elastic/kibana/assets/924948/0b729cd2-027b-4bc4-a481-4f8d8fa5e868">

Now introducing a basic `*` Filter as `split metric by`:

<img width="637" alt="Screenshot 2023-09-05 at 11 12 52"
src="https://github.com/elastic/kibana/assets/924948/31bf5814-1e30-4ff4-a9ec-1da60f9f7841">

And with a more complex filter (1098 + 564 = 1662):

<img width="624" alt="Screenshot 2023-09-05 at 11 12 36"
src="https://github.com/elastic/kibana/assets/924948/064f7496-7a90-4fbd-8ebf-a99c3a024125">




### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
dej611 and stratoula authored Sep 6, 2023
1 parent 4fc0369 commit 0f3209c
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,119 @@ describe('Terms Agg Other bucket helper', () => {

expect(agg).toEqual(false);
});

test('returns true when nested filter agg has buckets', () => {
const aggConfigs = getAggConfigs([
{
id: '0',
type: BUCKET_TYPES.FILTERS,
params: [
{
input: {
language: 'kuery',
query: '',
},
label: '',
},
],
},
...nestedTerm.aggs,
]);

const nestedTermResponseWithRootFilter = wrapResponse({
'0': {
buckets: {
'*': {
'1': {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 8325,
buckets: [
{
'2': {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 8325,
buckets: [
{ key: 'ios', doc_count: 2850 },
{ key: 'win xp', doc_count: 2830 },
{ key: '__missing__', doc_count: 1430 },
],
},
key: 'US-with-dash',
doc_count: 2850,
},
{
'2': {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 8325,
buckets: [
{ key: 'ios', doc_count: 1850 },
{ key: 'win xp', doc_count: 1830 },
{ key: '__missing__', doc_count: 130 },
],
},
key: 'IN-with-dash',
doc_count: 2830,
},
],
},
doc_count: 1148,
},
},
},
});

const otherAggConfig = buildOtherBucketAgg(
aggConfigs,
aggConfigs.aggs[2] as IBucketAggConfig,
enrichResponseWithSampling(nestedTermResponseWithRootFilter)
);

expect(otherAggConfig).toBeDefined();
if (otherAggConfig) {
const expectedResponse = {
'other-filter': {
aggs: undefined,
filters: {
filters: {
[`${SEP}*${SEP}IN-with-dash`]: {
bool: {
must: [],
filter: [
{ bool: { filter: [], must: [], must_not: [], should: [] } },
{ match_phrase: { 'geo.src': 'IN-with-dash' } },
{ exists: { field: 'machine.os.raw' } },
],
should: [],
must_not: [
{ match_phrase: { 'machine.os.raw': 'ios' } },
{ match_phrase: { 'machine.os.raw': 'win xp' } },
],
},
},
[`${SEP}*${SEP}US-with-dash`]: {
bool: {
must: [],
filter: [
{ bool: { filter: [], must: [], must_not: [], should: [] } },
{ match_phrase: { 'geo.src': 'US-with-dash' } },
{ exists: { field: 'machine.os.raw' } },
],
should: [],
must_not: [
{ match_phrase: { 'machine.os.raw': 'ios' } },
{ match_phrase: { 'machine.os.raw': 'win xp' } },
],
},
},
},
},
},
};
const resp = otherAggConfig();
const topAgg = !isSamplingEnabled(probability) ? resp : resp.sampling!.aggs;
expect(topAgg).toEqual(expectedResponse);
}
});
});

describe(`mergeOtherBucketAggResponse${getTitlePostfix()}`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ export const buildOtherBucketAgg = (
) => {
// make sure there are actually results for the buckets
const agg = aggregations[aggId];
if (!agg || !agg.buckets.length) {
if (
!agg ||
// buckets can be either an array or an object in case there's also a filter at the same level
(Array.isArray(agg.buckets) ? !agg.buckets.length : !Object.values(agg.buckets).length)
) {
noAggBucketResults = true;
return;
}
Expand Down

0 comments on commit 0f3209c

Please sign in to comment.