Skip to content

Commit

Permalink
[Lens] Fixes problem with timeshift in formula and breakdown (#150406)
Browse files Browse the repository at this point in the history
## Summary

Closes #146200

This bug can easily be replicated with the following scenario:
1. Add a date histogram on the x axis
2. Add a formula with shift (use different timeshifts) i.e. count() -
count(shift=1d)
3. Add a terms breakdown and switch off the Aggregate by this function
first

The fi is easy, it fails on the each look as it requires the agg to have
buckets but it doesnt at this scenario. I also added a test.

How it looks now
<img width="1049" alt="image"
src="https://user-images.githubusercontent.com/17003240/217195447-3b6edb8d-0814-4148-888d-9bd64e924c76.png">


### Checklist

- [ ] [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
  • Loading branch information
stratoula authored Feb 7, 2023
1 parent e05797b commit cf9707b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,38 @@ describe('Terms Agg Other bucket helper', () => {
expect(typeof agg).toBe('function');
});

test('returns a function for undefined agg buckets', () => {
const response = {
took: 10,
timed_out: false,
_shards: {
total: 1,
successful: 1,
skipped: 0,
failed: 0,
},
hits: {
total: 14005,
max_score: 0,
hits: [],
},
aggregations: {
2: {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 8325,
},
},
status: 200,
};
const aggConfigs = getAggConfigs(nestedTerm.aggs);
const agg = buildOtherBucketAgg(
aggConfigs,
aggConfigs.aggs[1] as IBucketAggConfig,
enrichResponseWithSampling(response)
);
expect(agg).toBe(false);
});

test('correctly builds query with single terms agg', () => {
const aggConfigs = getAggConfigs(singleTerm.aggs);
const agg = buildOtherBucketAgg(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,20 +196,19 @@ export const buildOtherBucketAgg = (
key: string
) => {
// make sure there are actually results for the buckets
if (aggregations[aggId]?.buckets.length < 1) {
const agg = aggregations[aggId];
if (!agg || !agg.buckets.length) {
noAggBucketResults = true;
return;
}

const agg = aggregations[aggId];
const newAggIndex = aggIndex + 1;
const newAgg = bucketAggs[newAggIndex];
const currentAgg = bucketAggs[aggIndex];
if (aggIndex === index && agg && agg.sum_other_doc_count > 0) {
exhaustiveBuckets = false;
}
if (aggIndex < index) {
each(agg.buckets, (bucket: any, bucketObjKey) => {
each(agg?.buckets, (bucket: any, bucketObjKey) => {
const bucketKey = currentAgg.getKey(
bucket,
isNumber(bucketObjKey) ? undefined : bucketObjKey
Expand Down

0 comments on commit cf9707b

Please sign in to comment.