Skip to content

Commit 78fb096

Browse files
bdonovan1Brian Donovan
andauthored
fix(chart): word cloud secondary sort prevents Druid TopN optimization when sort_by_metric enabled (#39073)
Co-authored-by: Brian Donovan <briand@netflix.com>
1 parent 0681800 commit 78fb096

4 files changed

Lines changed: 172 additions & 13 deletions

File tree

superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,16 @@ import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
2121
import { WordCloudFormData } from '../types';
2222

2323
export default function buildQuery(formData: WordCloudFormData) {
24-
const { metric, sort_by_metric, series, row_limit } = formData;
24+
const { metric, sort_by_metric, sort_by_series, series, row_limit } =
25+
formData;
2526
const orderby: QueryFormOrderBy[] = [];
2627
const shouldApplyOrderBy =
2728
row_limit !== undefined && row_limit !== null && row_limit !== 0;
2829

2930
if (sort_by_metric && metric) {
3031
orderby.push([metric, false]);
3132
}
32-
if (series) {
33+
if (sort_by_series && series) {
3334
orderby.push([series, true]);
3435
}
3536

superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/controlPanel.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ const config: ControlPanelConfig = {
3535
['adhoc_filters'],
3636
['row_limit'],
3737
['sort_by_metric'],
38+
[
39+
{
40+
name: 'sort_by_series',
41+
config: {
42+
type: 'CheckboxControl',
43+
label: t('Sort by series'),
44+
default: false,
45+
description: t(
46+
'Sort results by series name in ascending order. ' +
47+
'When combined with "Sort by metric", this acts as a tiebreaker ' +
48+
'for equal metric values. Adding this sort may reduce query ' +
49+
'performance on some databases.',
50+
),
51+
},
52+
},
53+
],
3854
],
3955
},
4056
{

superset-frontend/plugins/plugin-chart-word-cloud/test/buildQuery.test.ts

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,70 @@ import { VizType } from '@superset-ui/core';
2121
import { WordCloudFormData } from '../src';
2222
import buildQuery from '../src/plugin/buildQuery';
2323

24-
describe('WordCloud buildQuery', () => {
25-
const formData: WordCloudFormData = {
26-
datasource: '5__table',
27-
granularity_sqla: 'ds',
28-
series: 'foo',
29-
viz_type: VizType.WordCloud,
30-
};
24+
const basicFormData: WordCloudFormData = {
25+
datasource: '5__table',
26+
granularity_sqla: 'ds',
27+
series: 'foo',
28+
viz_type: VizType.WordCloud,
29+
};
3130

32-
test('should build columns from series in form data', () => {
33-
const queryContext = buildQuery(formData);
34-
const [query] = queryContext.queries;
35-
expect(query.columns).toEqual(['foo']);
31+
describe('plugin-chart-word-cloud', () => {
32+
describe('buildQuery', () => {
33+
test('should build columns from series in form data', () => {
34+
const queryContext = buildQuery(basicFormData);
35+
const [query] = queryContext.queries;
36+
expect(query.columns).toEqual(['foo']);
37+
});
38+
39+
test('should not include orderby when neither sort option is enabled', () => {
40+
const queryContext = buildQuery({
41+
...basicFormData,
42+
metric: 'count',
43+
sort_by_metric: false,
44+
sort_by_series: false,
45+
row_limit: 100,
46+
});
47+
const [query] = queryContext.queries;
48+
expect(query.orderby).toBeUndefined();
49+
});
50+
51+
test('should order by metric DESC only when sort_by_metric is true', () => {
52+
const queryContext = buildQuery({
53+
...basicFormData,
54+
metric: 'count',
55+
sort_by_metric: true,
56+
sort_by_series: false,
57+
row_limit: 100,
58+
});
59+
const [query] = queryContext.queries;
60+
expect(query.orderby).toEqual([['count', false]]);
61+
});
62+
63+
test('should order by series ASC only when sort_by_series is true', () => {
64+
const queryContext = buildQuery({
65+
...basicFormData,
66+
metric: 'count',
67+
sort_by_metric: false,
68+
sort_by_series: true,
69+
row_limit: 100,
70+
});
71+
const [query] = queryContext.queries;
72+
expect(query.orderby).toEqual([['foo', true]]);
73+
});
74+
75+
test('should order by metric DESC then series ASC when both are true', () => {
76+
const queryContext = buildQuery({
77+
...basicFormData,
78+
metric: 'count',
79+
sort_by_metric: true,
80+
sort_by_series: true,
81+
row_limit: 100,
82+
});
83+
const [query] = queryContext.queries;
84+
expect(query.orderby).toEqual([
85+
['count', false],
86+
['foo', true],
87+
]);
88+
});
3689
});
3790
});
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
"""add sort_by_series to word_cloud charts
18+
19+
Revision ID: fd0c8583b46d
20+
Revises: ce6bd21901ab
21+
Create Date: 2026-04-13 19:28:19.021839
22+
23+
"""
24+
25+
from alembic import op
26+
from sqlalchemy import Column, Integer, String, Text
27+
from sqlalchemy.ext.declarative import declarative_base
28+
29+
from superset import db
30+
from superset.migrations.shared.utils import paginated_update
31+
from superset.utils import json
32+
33+
# revision identifiers, used by Alembic.
34+
revision = "fd0c8583b46d"
35+
down_revision = "ce6bd21901ab"
36+
37+
Base = declarative_base()
38+
39+
40+
class Slice(Base):
41+
__tablename__ = "slices"
42+
id = Column(Integer, primary_key=True)
43+
viz_type = Column(String(250))
44+
params = Column(Text)
45+
46+
47+
def upgrade_params(params: dict) -> dict:
48+
if "sort_by_series" not in params:
49+
params["sort_by_series"] = True
50+
return params
51+
52+
53+
def downgrade_params(params: dict) -> dict:
54+
params.pop("sort_by_series", None)
55+
return params
56+
57+
58+
def upgrade():
59+
bind = op.get_bind()
60+
session = db.Session(bind=bind)
61+
62+
for slc in paginated_update(
63+
session.query(Slice).filter(Slice.viz_type == "word_cloud")
64+
):
65+
try:
66+
params = json.loads(slc.params or "{}")
67+
if not isinstance(params, dict):
68+
continue
69+
slc.params = json.dumps(upgrade_params(params))
70+
except (json.JSONDecodeError, TypeError):
71+
continue
72+
session.close()
73+
74+
75+
def downgrade():
76+
bind = op.get_bind()
77+
session = db.Session(bind=bind)
78+
79+
for slc in paginated_update(
80+
session.query(Slice).filter(Slice.viz_type == "word_cloud")
81+
):
82+
try:
83+
params = json.loads(slc.params or "{}")
84+
if not isinstance(params, dict):
85+
continue
86+
slc.params = json.dumps(downgrade_params(params))
87+
except (json.JSONDecodeError, TypeError):
88+
continue
89+
session.close()

0 commit comments

Comments
 (0)