From d8b4ed3a0e7f431119f7c873249e8c01409d97a2 Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Wed, 13 Jan 2021 09:04:36 +0000 Subject: [PATCH] Elasticsearch: fix handling of null values in query_builder (#30234) --- .../datasource/elasticsearch/query_builder.ts | 9 +++++---- .../elasticsearch/specs/query_builder.test.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/public/app/plugins/datasource/elasticsearch/query_builder.ts b/public/app/plugins/datasource/elasticsearch/query_builder.ts index f0180ad7fd8..82c08e6c60f 100644 --- a/public/app/plugins/datasource/elasticsearch/query_builder.ts +++ b/public/app/plugins/datasource/elasticsearch/query_builder.ts @@ -334,10 +334,11 @@ export class ElasticQueryBuilder { metricAgg = { field: metric.field }; } - metricAgg = { - ...metricAgg, - ...(isMetricAggregationWithSettings(metric) && metric.settings), - }; + if (isMetricAggregationWithSettings(metric)) { + Object.entries(metric.settings || {}) + .filter(([_, v]) => v !== null) + .forEach(([k, v]) => (metricAgg[k] = v)); + } aggField[metric.type] = metricAgg; nestedAggs.aggs[metric.id] = aggField; diff --git a/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts b/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts index 2f47a82b6b3..f98123d4dee 100644 --- a/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts +++ b/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts @@ -24,6 +24,22 @@ describe('ElasticQueryBuilder', () => { expect(query.aggs['1'].date_histogram.extended_bounds.min).toBe('$timeFrom'); }); + it('should clean settings from null values', () => { + const query = builder.build({ + refId: 'A', + // The following `missing: null as any` is because previous versions of the DS where + // storing null in the query model when inputting an empty string, + // which were then removed in the query builder. + // The new version doesn't store empty strings at all. This tests ensures backward compatinility. + metrics: [{ type: 'avg', id: '0', settings: { missing: null as any, script: '1' } }], + timeField: '@timestamp', + bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '1' }], + }); + + expect(query.aggs['1'].aggs['0'].avg.missing).not.toBeDefined(); + expect(query.aggs['1'].aggs['0'].avg.script).toBeDefined(); + }); + it('with multiple bucket aggs', () => { const query = builder.build({ refId: 'A',