You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/06/02 17:49:50 UTC

(superset) branch master updated: test(histogram): metric filters require aggregation in buildQuery (#30330) (#40617)

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 24422c83114 test(histogram): metric filters require aggregation in buildQuery (#30330) (#40617)
24422c83114 is described below

commit 24422c831146d0beac5bf89eac7098e7328b51cb
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Tue Jun 2 10:49:43 2026 -0700

    test(histogram): metric filters require aggregation in buildQuery (#30330) (#40617)
    
    Co-authored-by: Claude Sonnet 4.6 <no...@anthropic.com>
---
 .../src/Histogram/buildQuery.ts                    | 15 +++-
 .../test/Histogram/buildQuery.test.ts              | 81 ++++++++++++++++++++++
 2 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/buildQuery.ts
index 4afcb1e4af3..493a34d556f 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/buildQuery.ts
@@ -21,13 +21,24 @@ import { histogramOperator } from '@superset-ui/chart-controls';
 import { HistogramFormData } from './types';
 
 export default function buildQuery(formData: HistogramFormData) {
-  const { column, groupby = [] } = formData;
+  const { column, groupby = [], adhoc_filters } = formData;
+  const hasHavingFilter = (adhoc_filters ?? []).some(
+    (filter: { clause?: string }) => filter.clause === 'HAVING',
+  );
   return buildQueryContext(formData, baseQueryObject => [
     {
       ...baseQueryObject,
       columns: [...groupby, column],
       post_processing: [histogramOperator(formData, baseQueryObject)],
-      metrics: undefined,
+      metrics: hasHavingFilter
+        ? [
+            {
+              expressionType: 'SQL' as const,
+              sqlExpression: 'COUNT(*)',
+              label: 'COUNT(*)',
+            },
+          ]
+        : undefined,
     },
   ]);
 }
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Histogram/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Histogram/buildQuery.test.ts
new file mode 100644
index 00000000000..1726b02f983
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Histogram/buildQuery.test.ts
@@ -0,0 +1,81 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import buildQuery from '../../src/Histogram/buildQuery';
+import { HistogramFormData } from '../../src/Histogram/types';
+
+const baseFormData: HistogramFormData = {
+  datasource: '5__table',
+  granularity_sqla: 'ds',
+  column: 'price',
+  groupby: [],
+  bins: 10,
+  viz_type: 'histogram',
+  cumulative: false,
+  normalize: false,
+  sliceId: 1,
+  showLegend: false,
+  showValue: false,
+  xAxisFormat: '',
+  xAxisTitle: '',
+  yAxisFormat: '',
+  yAxisTitle: '',
+};
+
+test('should build query with column and no metrics', () => {
+  const queryContext = buildQuery(baseFormData);
+  const [query] = queryContext.queries;
+  expect(query.columns).toContain('price');
+  expect(query.metrics).toBeUndefined();
+});
+
+test('should include groupby columns in query columns', () => {
+  const queryContext = buildQuery({ ...baseFormData, groupby: ['category'] });
+  const [query] = queryContext.queries;
+  expect(query.columns).toEqual(['category', 'price']);
+});
+
+test('Regression for #30330: HAVING-clause metric filters require aggregation in the query', () => {
+  /**
+   * buildQuery unconditionally sets metrics: undefined, which means any
+   * HAVING-clause adhoc_filter produces SQL with a HAVING clause but no
+   * GROUP BY or aggregated metric — invalid SQL that most databases reject.
+   *
+   * The fix should preserve (or synthesise) a metric so the HAVING clause
+   * has an aggregated value to filter on. This test asserts that desired
+   * behaviour: when a HAVING adhoc_filter is present, query.metrics must
+   * not be undefined or empty.
+   */
+  const formDataWithHavingFilter: HistogramFormData = {
+    ...baseFormData,
+    adhoc_filters: [
+      {
+        clause: 'HAVING',
+        expressionType: 'SQL',
+        sqlExpression: 'COUNT(*) > 5',
+      },
+    ],
+  };
+  const queryContext = buildQuery(formDataWithHavingFilter);
+  const [query] = queryContext.queries;
+
+  // HAVING filters without aggregation produce invalid SQL.
+  // The query must include at least one metric when HAVING filters are present.
+  expect(query.metrics).toBeDefined();
+  expect((query.metrics as unknown[]).length).toBeGreaterThan(0);
+});