You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ms...@apache.org on 2026/08/28 13:46:16 UTC
(superset) branch master updated: feat(filters): search filter values server-side in Explore (#43518)
This is an automated email from the ASF dual-hosted git repository.
msyavuz 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 abf338d611d feat(filters): search filter values server-side in Explore (#43518)
abf338d611d is described below
commit abf338d611de6bb2ea83890cf2e1481e145d7b16
Author: Mehmet Salih Yavuz <sa...@proton.me>
AuthorDate: Fri Aug 28 16:46:09 2026 +0300
feat(filters): search filter values server-side in Explore (#43518)
Co-authored-by: Enzo Martellucci <52...@users.noreply.github.com>
---
...AdhocFilterEditPopoverSimpleTabContent.test.tsx | 164 +++++++++++++
.../index.tsx | 264 ++++++++++++++-------
.../components/Select/SelectFilterPlugin.test.tsx | 35 ++-
.../components/Select/SelectFilterPlugin.tsx | 36 ++-
.../filters/components/Select/buildQuery.test.ts | 32 +++
.../src/filters/components/Select/buildQuery.ts | 9 +-
superset/connectors/sqla/models.py | 1 +
superset/datasource/api.py | 25 +-
superset/models/helpers.py | 39 +++
tests/integration_tests/datasource/api_tests.py | 75 ++++++
tests/unit_tests/models/helpers_test.py | 80 +++++++
11 files changed, 657 insertions(+), 103 deletions(-)
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
index 8e0f9f16ae8..3bb8c6d9da4 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
@@ -951,3 +951,167 @@ test('filters the subject select by column verbose_name as well as column_name',
expect(within(dropdown).getByText('total_count')).toBeInTheDocument();
expect(within(dropdown).queryByText('Full Name')).not.toBeInTheDocument();
});
+
+const COLUMN_VALUES_ENDPOINT =
+ 'glob:*/api/v1/datasource/*/column/value/values/*';
+
+let columnValues: { result: unknown[]; limit: number } = {
+ result: [],
+ limit: 10000,
+};
+fetchMock.get(COLUMN_VALUES_ENDPOINT, () => columnValues);
+
+const setupWithFilterValues = (result: unknown[], limit = 10000) => {
+ columnValues = { result, limit };
+ const onChange = jest.fn();
+ const validHandler = jest.fn();
+ const spy = jest.spyOn(redux, 'useSelector');
+ spy.mockReturnValue({});
+ const props = {
+ adhocFilter: new AdhocFilter({
+ expressionType: ExpressionTypes.Simple,
+ subject: 'value',
+ operatorId: Operators.In,
+ operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
+ comparator: [],
+ clause: Clauses.Where,
+ }),
+ onChange,
+ options,
+ datasource: {
+ ...TestDataset,
+ columns: [{ column_name: 'value', type: 'VARCHAR', id: 3 }],
+ filter_select: true,
+ },
+ partitionColumn: 'test',
+ validHandler,
+ };
+ render(
+ <AdhocFilterEditPopoverSimpleTabContent {...(props as unknown as Props)} />,
+ );
+ return props;
+};
+
+const openComparator = async () => {
+ const comparator = screen.getByRole('combobox', {
+ name: 'Comparator option',
+ });
+ userEvent.click(comparator);
+ return comparator;
+};
+
+test('loads comparator values from the server', async () => {
+ setupWithFilterValues(['alpha', 'beta']);
+ await openComparator();
+ expect(await screen.findByTitle('alpha')).toBeInTheDocument();
+});
+
+test('sends the typed text to the server rather than filtering the loaded page', async () => {
+ // The loaded page is bounded, so matching client-side cannot reach a value
+ // beyond the row limit. The search has to reach the database.
+ setupWithFilterValues(['alpha']);
+ const comparator = await openComparator();
+ userEvent.type(comparator, 'gamma');
+
+ await waitFor(
+ () => {
+ const searched = fetchMock.callHistory
+ .calls(COLUMN_VALUES_ENDPOINT)
+ .map(call => String(call.url));
+ expect(searched.some(url => url.includes('q=gamma'))).toBe(true);
+ },
+ { timeout: 3000 },
+ );
+});
+
+test('lets a value the server did not return still be selected', async () => {
+ // Even with server-side search a match can fall outside the page; typing the
+ // exact value has to remain a way through.
+ setupWithFilterValues([]);
+ const comparator = await openComparator();
+ userEvent.type(comparator, 'not-in-the-page');
+ expect(await screen.findByTitle('not-in-the-page')).toBeInTheDocument();
+});
+
+test('does not query for values when the dataset disables them', async () => {
+ fetchMock.clearHistory();
+ setup({
+ adhocFilter: new AdhocFilter({
+ expressionType: ExpressionTypes.Simple,
+ subject: 'value',
+ operatorId: Operators.In,
+ operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
+ comparator: [],
+ clause: Clauses.Where,
+ }),
+ });
+ await openComparator();
+ expect(fetchMock.callHistory.calls(COLUMN_VALUES_ENDPOINT)).toHaveLength(0);
+});
+
+test('stores the picked value, not the option object', async () => {
+ // AsyncSelect is labelInValue: taking its argument at face value puts
+ // {label, value} into the comparator, and the engine then fails to render it
+ // as a literal.
+ const props = setupWithFilterValues(['Michael']);
+ await openComparator();
+ userEvent.click(await screen.findByTitle('Michael'));
+
+ await waitFor(() => expect(props.onChange).toHaveBeenCalled());
+ const [filter] = props.onChange.mock.calls.at(-1);
+ expect(filter.comparator).toEqual(['Michael']);
+});
+
+test('can remove a value that was saved earlier', async () => {
+ // Reopening the popover restores the comparator from the saved filter, and
+ // the value is not in the freshly loaded page. Removing it has to still work.
+ columnValues = { result: [], limit: 10000 };
+ const onChange = jest.fn();
+ const validHandler = jest.fn();
+ jest.spyOn(redux, 'useSelector').mockReturnValue({});
+ render(
+ <AdhocFilterEditPopoverSimpleTabContent
+ {...({
+ adhocFilter: new AdhocFilter({
+ expressionType: ExpressionTypes.Simple,
+ subject: 'value',
+ operatorId: Operators.In,
+ operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.In].operation,
+ comparator: ['Michael'],
+ clause: Clauses.Where,
+ }),
+ onChange,
+ options,
+ datasource: {
+ ...TestDataset,
+ columns: [{ column_name: 'value', type: 'VARCHAR', id: 3 }],
+ filter_select: true,
+ },
+ partitionColumn: 'test',
+ validHandler,
+ } as unknown as Props)}
+ />,
+ );
+
+ // Remove it the way a user does: the tag's own close control.
+ userEvent.click(await screen.findByLabelText('close'));
+
+ await waitFor(() => expect(onChange).toHaveBeenCalled());
+ const [filter] = onChange.mock.calls.at(-1);
+ expect(filter.comparator).toEqual([]);
+});
+
+test('says the list is partial when the server capped it', async () => {
+ setupWithFilterValues(['alpha', 'beta'], 2);
+ await openComparator();
+ expect(
+ await screen.findByText(/Only the first 2 values are listed/),
+ ).toBeInTheDocument();
+});
+
+test('does not say the list is partial when it is complete', async () => {
+ setupWithFilterValues(['alpha', 'beta'], 10000);
+ await openComparator();
+ expect(await screen.findByTitle('alpha')).toBeInTheDocument();
+ expect(screen.queryByText(/Only the first/)).not.toBeInTheDocument();
+});
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
index 692d84b3135..631d743bd3c 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
@@ -16,13 +16,25 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { FC, ChangeEvent, useEffect, useState, useRef } from 'react';
+import {
+ FC,
+ ChangeEvent,
+ useCallback,
+ useEffect,
+ useMemo,
+ useState,
+ useRef,
+} from 'react';
import {
+ AsyncSelect,
Input,
InputRef,
Select,
Tooltip,
+ type AsyncSelectRef,
+ type LabeledValue,
+ type SelectOptionsTypePage,
type SelectValue,
} from '@superset-ui/core/components';
import { t } from '@apache-superset/core/translation';
@@ -57,7 +69,7 @@ import { useDatePickerInAdhocFilter } from '../utils';
import { useDefaultTimeFilter } from '../../DateFilterControl/utils';
import { Clauses, ExpressionTypes } from '../types';
-const SelectWithLabel = styled(Select)<{ labelText: string }>`
+const SelectWithLabel = styled(AsyncSelect)<{ labelText: string }>`
.ant-select-content::after {
content: ${({ labelText }) => labelText || '\\A0'};
display: inline-block;
@@ -67,6 +79,30 @@ const SelectWithLabel = styled(Select)<{ labelText: string }>`
}
`;
+// The server answers with one bounded page, not an offset window: paging would
+// need a stable ORDER BY, and ordering a high-cardinality column is the full
+// scan this search exists to avoid. A page size no response can reach keeps
+// AsyncSelect from asking for a second page.
+const COMPARATOR_PAGE_SIZE = 1_000_000;
+
+const toLabeledValue = (value: unknown): LabeledValue => ({
+ value: value as LabeledValue['value'],
+ label: optionLabel(value as null | number | boolean | string),
+});
+
+// The reverse of toLabeledValue: what AsyncSelect emits is labelled, and the
+// comparator has to be the raw value or the engine cannot render it as a
+// literal.
+const unwrapComparator = (value: unknown): unknown => {
+ if (Array.isArray(value)) {
+ return value.map(unwrapComparator);
+ }
+ if (value !== null && typeof value === 'object' && 'value' in value) {
+ return (value as LabeledValue).value;
+ }
+ return value;
+};
+
export interface SimpleExpressionType {
expressionType: keyof typeof ExpressionTypes;
column: ColumnMeta;
@@ -347,11 +383,9 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
} = useSimpleTabFilterProps(props);
const [comparator, setComparator] = useState(props.adhocFilter.comparator);
const comparatorInputRef = useRef<InputRef | null>(null);
- const [suggestions, setSuggestions] = useState<
- Record<'label' | 'value', any>[]
- >([]);
- const [loadingComparatorSuggestions, setLoadingComparatorSuggestions] =
- useState<boolean>(false);
+ const comparatorSelectRef = useRef<AsyncSelectRef>(null);
+ const [loadedOptionCount, setLoadedOptionCount] = useState(0);
+ const [optionsTruncated, setOptionsTruncated] = useState(false);
const [hasFocusedComparator, setHasFocusedComparator] =
useState<boolean>(false);
@@ -387,18 +421,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
/>
);
- const getOptionsRemaining = () => {
- // if select is multi/value is array, we show the options not selected
- const valuesFromSuggestionsLength = Array.isArray(comparator)
- ? comparator.filter(v => suggestions.includes(v)).length
- : 0;
- return suggestions ? suggestions.length - valuesFromSuggestionsLength : 0;
- };
- const createSuggestionsPlaceholder = () => {
- const optionsRemaining = getOptionsRemaining();
- const placeholder = t('%s option(s)', optionsRemaining);
- return optionsRemaining ? placeholder : '';
- };
+ const createSuggestionsPlaceholder = () =>
+ loadedOptionCount ? t('%s option(s)', loadedOptionCount) : '';
const handleSubjectChange = (subject: string) => {
setComparator(undefined);
@@ -455,21 +479,63 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
operatorId !== undefined &&
DISABLE_INPUT_OPERATORS.includes(operatorId as Operators);
+ const canSuggestComparatorValues = Boolean(
+ subjectString &&
+ props.datasource?.filter_select &&
+ props.adhocFilter.clause !== Clauses.Having,
+ );
+
const hasComparatorOptions =
(operatorId && MULTI_OPERATORS.has(operatorId as Operators)) ||
- suggestions.length > 0;
+ canSuggestComparatorValues;
+
+ // AsyncSelect is labelInValue, so the value it is given has to be labelled
+ // too. Handed a bare value it still renders, but `handleOnDeselect` then
+ // compares `element.value` against entries that have no `.value`, matches
+ // nothing, and the tag cannot be removed.
+ //
+ // Memoised because AsyncSelect resets its internal selection whenever the
+ // identity of `value` changes. A fresh array every render would wipe out
+ // each pick as soon as it was made.
+ const comparatorSelectValue = useMemo(
+ () =>
+ Array.isArray(comparator)
+ ? comparator.map(toLabeledValue)
+ : isDefined(comparator) && comparator !== ''
+ ? toLabeledValue(comparator)
+ : undefined,
+ [comparator],
+ );
+
+ const handleComparatorChange = useCallback(
+ (value: unknown) => {
+ onComparatorChange(unwrapComparator(value) as string);
+ },
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ [props.adhocFilter, props.onChange],
+ );
const comparatorSelectProps = {
allowClear: true,
allowNewOptions: true,
ariaLabel: t('Comparator option'),
+ pageSize: COMPARATOR_PAGE_SIZE,
+ // A capped list reads as the whole set unless it says otherwise, so an
+ // absent value looks like a value that does not exist. Only shown when the
+ // list is actually cut short.
+ helperText: optionsTruncated
+ ? t(
+ 'Only the first %s values are listed. Type to search all of them, ' +
+ 'or enter a value that is not listed.',
+ loadedOptionCount,
+ )
+ : undefined,
mode:
operatorId && MULTI_OPERATORS.has(operatorId as Operators)
? ('multiple' as const)
: ('single' as const),
- loading: loadingComparatorSuggestions,
- value: comparator as SelectValue,
- onChange: onComparatorChange,
+ value: comparatorSelectValue as SelectValue,
+ onChange: handleComparatorChange,
notFoundContent: t('Type a value here'),
placeholder: createSuggestionsPlaceholder(),
};
@@ -495,76 +561,89 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
onChange: onDatePickerChange,
});
- useEffect(() => {
- const refreshComparatorSuggestions = () => {
- const { datasource } = props;
- const col = props.adhocFilter.subject;
- const having = props.adhocFilter.clause === Clauses.Having;
-
- if (col && datasource && datasource.filter_select && !having) {
- const controller = new AbortController();
- const { signal } = controller;
- if (loadingComparatorSuggestions) {
- controller.abort();
- }
- // Element-level array operators (Contains any / Contains all) search
- // inside the array, so suggest individual elements; whole-array
- // operators (=, In, …) keep the default distinct-array suggestions.
- const { operatorId } = props.adhocFilter;
- const arrayElements =
- operatorId === Operators.ContainsAny ||
- operatorId === Operators.ContainsAll;
- setLoadingComparatorSuggestions(true);
- SupersetClient.get({
- signal,
- endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${
- arrayElements ? '?array_elements=true' : ''
- }`,
- })
- .then(({ json }) => {
- setSuggestions(
- json.result.map((suggestion: unknown) => {
- // Complex column values arrive as JS arrays or objects: whole
- // arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple
- // objects for nested-container columns (e.g. {"a": ["x","y"]}).
- // A raw array/object is neither a valid single-select value
- // (antd collapses an array to its first element) nor renderable
- // as a React child (an object throws). Render it as its literal
- // string, which is also exactly what the backend's
- // parse_array_literal expects for the whole-array operators.
- if (suggestion !== null && typeof suggestion === 'object') {
- const literal = JSON.stringify(suggestion);
- return { value: literal, label: literal };
- }
- return {
- value: suggestion as null | number | boolean | string,
- label: optionLabel(
- suggestion as null | number | boolean | string,
- ),
- };
- }),
- );
- setLoadingComparatorSuggestions(false);
- })
- .catch(() => {
- setSuggestions([]);
- setLoadingComparatorSuggestions(false);
- });
+ // Element-level array operators (Contains any / Contains all) search inside
+ // the array, so suggest individual elements; whole-array operators (=, In, …)
+ // keep the default distinct-array suggestions.
+ const arrayElements =
+ props.adhocFilter.operatorId === Operators.ContainsAny ||
+ props.adhocFilter.operatorId === Operators.ContainsAll;
+
+ // AsyncSelect throws away every loaded option when the identity of its
+ // `options` callback changes, so this depends on plain values rather than on
+ // `props.datasource`, whose identity the parent does not guarantee.
+ const datasourceType = props.datasource?.type;
+ const datasourceId = props.datasource?.id;
+
+ const loadComparatorOptions = useCallback(
+ async (search: string): Promise<SelectOptionsTypePage> => {
+ const col = subjectString;
+ if (!col || !canSuggestComparatorValues) {
+ return { data: [], totalCount: 0 };
}
- };
- if (!datePicker) {
- refreshComparatorSuggestions();
- }
- // loadingComparatorSuggestions intentionally omitted - set inside effect, would cause infinite loop
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [
- props.adhocFilter.subject,
- props.adhocFilter.clause,
- props.adhocFilter.operatorId,
- props.datasource,
- datePicker,
- ]);
+ const params = new URLSearchParams();
+ if (arrayElements) {
+ params.set('array_elements', 'true');
+ }
+ if (search) {
+ params.set('q', search);
+ }
+ const query = params.toString();
+
+ try {
+ const { json } = await SupersetClient.get({
+ endpoint:
+ `/api/v1/datasource/${datasourceType}/${datasourceId}` +
+ `/column/${encodeURIComponent(col)}/values/${query ? `?${query}` : ''}`,
+ });
+ const data = json.result.map((suggestion: unknown) => {
+ // Complex column values arrive as JS arrays or objects: whole arrays
+ // for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple objects for
+ // nested-container columns (e.g. {"a": ["x","y"]}). A raw
+ // array/object is neither a valid single-select value (antd collapses
+ // an array to its first element) nor renderable as a React child (an
+ // object throws). Render it as its literal string, which is also
+ // exactly what the backend's parse_array_literal expects for the
+ // whole-array operators.
+ if (suggestion !== null && typeof suggestion === 'object') {
+ const literal = JSON.stringify(suggestion);
+ return { value: literal, label: literal };
+ }
+ return {
+ value: suggestion as null | number | boolean | string,
+ label: optionLabel(suggestion as null | number | boolean | string),
+ };
+ });
+
+ setLoadedOptionCount(data.length);
+ setOptionsTruncated(isDefined(json.limit) && data.length >= json.limit);
+
+ // The count has to exceed what was returned. AsyncSelect treats
+ // `loaded >= totalCount` as "that is every value", sets allValuesLoaded
+ // and from then on serves searches by filtering the loaded page
+ // client-side -- which is the behaviour this whole change exists to
+ // replace. Pagination is held off by COMPARATOR_PAGE_SIZE instead.
+ return { data, totalCount: data.length + 1 };
+ } catch {
+ setLoadedOptionCount(0);
+ setOptionsTruncated(false);
+ return { data: [], totalCount: 0 };
+ }
+ },
+ [
+ subjectString,
+ canSuggestComparatorValues,
+ datasourceType,
+ datasourceId,
+ arrayElements,
+ ],
+ );
+
+ // Options are cached per search term inside AsyncSelect; a different column
+ // or a switch to element-level suggestions invalidates all of them.
+ useEffect(() => {
+ comparatorSelectRef.current?.clearCache();
+ }, [subjectString, arrayElements]);
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
@@ -670,11 +749,12 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
}
>
<SelectWithLabel
+ ref={comparatorSelectRef}
css={css`
margin-top: ${theme.marginXS}px;
`}
labelText={labelText}
- options={suggestions}
+ options={loadComparatorOptions}
{...comparatorSelectProps}
/>
</Tooltip>
diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
index 39d1ed3756b..bca8c575490 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
@@ -879,10 +879,41 @@ describe('SelectFilterPlugin', () => {
expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
});
- test('does not show create option when searchAllOptions is true', () => {
+ test('says the list is capped when it hits the row limit', async () => {
+ // 3 rows of data against a limit of 3: the user is looking at a page, not
+ // at every value the column has.
+ getWrapper({ rowLimit: 3 });
+ userEvent.click(screen.getAllByRole('combobox')[0]);
+ expect(
+ await screen.findByText(/Only the first 3 values are listed/),
+ ).toBeInTheDocument();
+ });
+
+ test('offers the ways out that the filter actually supports', async () => {
+ getWrapper({ rowLimit: 3, creatable: true, searchAllOptions: true });
+ userEvent.click(screen.getAllByRole('combobox')[0]);
+ expect(
+ await screen.findByText(/Type to search all of them/),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByText(/You can enter a value that is not listed/),
+ ).toBeInTheDocument();
+ });
+
+ test('says nothing when the whole column fits under the limit', async () => {
+ getWrapper();
+ userEvent.click(screen.getAllByRole('combobox')[0]);
+ expect(await screen.findByRole('combobox')).toBeInTheDocument();
+ expect(screen.queryByText(/Only the first/)).not.toBeInTheDocument();
+ });
+
+ test('shows create option when searchAllOptions is true', async () => {
+ // Server-side search returns a bounded page, so a value that exists in the
+ // data can still be missing from the dropdown. Suppressing the create
+ // option there leaves the user with no way to apply it at all.
getWrapper({ creatable: true, searchAllOptions: true });
userEvent.type(screen.getByRole('combobox'), 'brand-new');
- expect(screen.queryByTitle('brand-new')).not.toBeInTheDocument();
+ expect(await screen.findByTitle('brand-new')).toBeInTheDocument();
});
});
diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
index d1739c857e6..0737e7c7ef9 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@@ -271,7 +271,10 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
type: 'ownState',
ownState: {
coltypeMap: initialColtypeMap,
- search,
+ // The dropdown offers `stripSurroundingQuotes(search)` as the
+ // creatable option, so the server has to be asked for the same
+ // string or the two disagree about what was searched for.
+ search: stripSurroundingQuotes(search).trim(),
},
});
}
@@ -281,8 +284,10 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const handleBlur = useCallback(() => {
unsetFocusedFilter();
- onSearch('');
- }, [onSearch, unsetFocusedFilter]);
+ if (search) {
+ onSearch('');
+ }
+ }, [onSearch, search, unsetFocusedFilter]);
const handleChange = useCallback(
(value?: SelectValue | number | string) => {
@@ -304,6 +309,25 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
? t('No data')
: tn('%s option', '%s options', data.length, data.length);
+ // A capped list reads as the whole set, so a value sitting past the row
+ // limit looks like a value that does not exist. Each sentence is only added
+ // when it is actually true of this filter's configuration.
+ const rowLimit = Number(formData.rowLimit) || 0;
+ const helperText = useMemo(() => {
+ if (!rowLimit || data.length < rowLimit) {
+ return undefined;
+ }
+ return [
+ t('Only the first %s values are listed.', data.length),
+ searchAllOptions ? t('Type to search all of them.') : undefined,
+ creatable !== false
+ ? t('You can enter a value that is not listed.')
+ : undefined,
+ ]
+ .filter(Boolean)
+ .join(' ');
+ }, [creatable, data.length, rowLimit, searchAllOptions]);
+
const formItemExtra = useMemo(() => {
if (filterState.validateMessage) {
return (
@@ -336,7 +360,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
const unquotedSearch = stripSurroundingQuotes(search);
if (
unquotedSearch &&
- !searchAllOptions &&
creatable !== false &&
!hasOption(unquotedSearch, uniqueOptions, true)
) {
@@ -346,7 +369,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
];
}
return uniqueOptions;
- }, [search, uniqueOptions, creatable, searchAllOptions]);
+ }, [search, uniqueOptions, creatable]);
const sortComparator = useCallback(
(a: LabeledValue, b: LabeledValue) => {
@@ -617,7 +640,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
name={formData.nativeFilterId}
allowClear
autoClearSearchValue
- allowNewOptions={!searchAllOptions && creatable !== false}
+ allowNewOptions={creatable !== false}
allowNewOptionsOnPaste={multiSelect && searchAllOptions}
allowSelectAll={!searchAllOptions}
value={multiSelect ? filterState.value || [] : filterState.value}
@@ -626,6 +649,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
showSearch={showSearch}
mode={multiSelect ? 'multiple' : 'single'}
placeholder={placeholderText}
+ helperText={helperText}
onClear={() => onSearch('')}
onSearch={onSearch}
onBlur={handleBlur}
diff --git a/superset-frontend/src/filters/components/Select/buildQuery.test.ts b/superset-frontend/src/filters/components/Select/buildQuery.test.ts
index 2870543ee6c..a3557df4463 100644
--- a/superset-frontend/src/filters/components/Select/buildQuery.test.ts
+++ b/superset-frontend/src/filters/components/Select/buildQuery.test.ts
@@ -117,6 +117,38 @@ describe('Select buildQuery', () => {
]);
});
+ test('should not sort by the searched column', () => {
+ // Ordering by a high-cardinality column makes the engine sort every match
+ // before applying the row limit; the dropdown re-sorts the page anyway.
+ const queryContext = buildQuery(
+ { ...formData, sortAscending: true },
+ {
+ ownState: {
+ search: 'abc',
+ coltypeMap: { my_col: GenericDataType.String },
+ },
+ },
+ );
+ const [query] = queryContext.queries;
+ expect(query.orderby).toEqual([]);
+ });
+
+ test('should keep the sort metric while searching', () => {
+ // A sort metric decides which rows come back, so dropping it would change
+ // the result set rather than just its order.
+ const queryContext = buildQuery(
+ { ...formData, sortMetric: 'my_metric', sortAscending: false },
+ {
+ ownState: {
+ search: 'abc',
+ coltypeMap: { my_col: GenericDataType.String },
+ },
+ },
+ );
+ const [query] = queryContext.queries;
+ expect(query.orderby).toEqual([['my_metric', false]]);
+ });
+
test('should add text search parameter for numeric to query filter', () => {
const queryContext = buildQuery(formData, {
ownState: {
diff --git a/superset-frontend/src/filters/components/Select/buildQuery.ts b/superset-frontend/src/filters/components/Select/buildQuery.ts
index 1e80a32483a..e9b404119ac 100644
--- a/superset-frontend/src/filters/components/Select/buildQuery.ts
+++ b/superset-frontend/src/filters/components/Select/buildQuery.ts
@@ -54,6 +54,13 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
}
const sortColumns = sortMetric ? [sortMetric] : columns;
+ // Sorting by the searched column makes the engine scan and sort every
+ // match before applying the row limit, which is the dominant cost of
+ // search-as-you-type on a high-cardinality column. The dropdown re-sorts
+ // the returned page client-side, so the server sort buys nothing here. A
+ // sort metric is different: it selects *which* rows come back, so it has
+ // to stay.
+ const skipOrderBy = !!search && !sortMetric;
const query: QueryObject[] = [
{
...baseQueryObject,
@@ -61,7 +68,7 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
metrics: sortMetric ? [sortMetric] : [],
filters: filters.concat(extraFilters),
orderby:
- sortMetric || sortAscending !== undefined
+ !skipOrderBy && (sortMetric || sortAscending !== undefined)
? sortColumns.map(column => [column, !!sortAscending])
: [],
},
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index b053cc63561..2409c2ffae4 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -964,6 +964,7 @@ class AnnotationDatasource(BaseDatasource):
limit: int = 10000,
denormalize_column: bool = False,
array_elements: bool = False,
+ search: str | None = None,
) -> list[Any]:
raise NotImplementedError()
diff --git a/superset/datasource/api.py b/superset/datasource/api.py
index 4e09e3edad8..a9f3bc9d351 100644
--- a/superset/datasource/api.py
+++ b/superset/datasource/api.py
@@ -41,6 +41,9 @@ from superset.views.base_api import BaseSupersetApi, statsd_metrics
logger = logging.getLogger(__name__)
+# Cache lifetime for search-filtered column values, in seconds.
+SEARCH_CACHE_TIMEOUT = 60
+
class DatasourceRestApi(BaseSupersetApi):
allow_browser_login = True
@@ -87,6 +90,14 @@ class DatasourceRestApi(BaseSupersetApi):
type: string
name: column_name
description: The name of the column to get values for
+ - in: query
+ schema:
+ type: string
+ name: q
+ description: >-
+ Optional case-insensitive substring; only values containing it are
+ returned. Lets the client search the full column rather than the
+ truncated first page.
responses:
200:
description: A List of distinct values for the column
@@ -136,6 +147,10 @@ class DatasourceRestApi(BaseSupersetApi):
# Element-level operators (Contains any / Contains all) request the
# distinct array *elements* rather than distinct whole arrays.
array_elements = parse_boolean_string(request.args.get("array_elements"))
+ # Server-side search. Without it the client can only match against the
+ # bounded first page, so a value beyond ``FILTER_SELECT_ROW_LIMIT`` is
+ # unfindable on a high-cardinality column.
+ search = (request.args.get("q") or "").strip() or None
# Cache distinct column-value results so a dashboard with many filters
# backed by the same (often heavy) virtual dataset doesn't re-execute
@@ -169,6 +184,7 @@ class DatasourceRestApi(BaseSupersetApi):
"limit": row_limit,
"denorm": denormalize_column,
"elements": array_elements,
+ "q": search,
"rls": security_manager.get_rls_cache_key(datasource),
"changed_on": str(getattr(datasource, "changed_on", "")),
},
@@ -184,7 +200,7 @@ class DatasourceRestApi(BaseSupersetApi):
logger.debug(
"column-values cache HIT: uid=%s col=%s", datasource.uid, column_name
)
- response = self.response(200, result=cached)
+ response = self.response(200, result=cached, limit=row_limit)
response.headers["X-Cache-Status"] = "HIT"
return response
@@ -194,6 +210,7 @@ class DatasourceRestApi(BaseSupersetApi):
limit=row_limit,
denormalize_column=denormalize_column,
array_elements=array_elements,
+ search=search,
)
except KeyError:
return self.response(
@@ -225,11 +242,15 @@ class DatasourceRestApi(BaseSupersetApi):
timeout = datasource.cache_timeout or app.config.get(
"CACHE_DEFAULT_TIMEOUT", 300
)
+ if search:
+ # Every distinct search term is its own key, so a few users typing
+ # would otherwise pin one entry per keystroke for the full timeout.
+ timeout = min(timeout, SEARCH_CACHE_TIMEOUT)
cache_manager.data_cache.set(cache_key, payload, timeout=timeout)
logger.debug(
"column-values cache MISS: uid=%s col=%s", datasource.uid, column_name
)
- response = self.response(200, result=payload)
+ response = self.response(200, result=payload, limit=row_limit)
response.headers["X-Cache-Status"] = "MISS"
return response
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index e4dc83f0552..257a8344ef4 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -198,6 +198,41 @@ def get_effective_hours_offset(
R_SUFFIX = "__right_suffix"
+# Escape character for LIKE patterns built from user-supplied search text.
+# Deliberately not a backslash: dialects that escape backslashes when rendering
+# string literals would emit a two-character ESCAPE clause, which is a syntax
+# error on engines that honour standard-conforming strings.
+LIKE_ESCAPE_CHAR = "!"
+
+
+def escape_like_pattern(value: str) -> str:
+ """
+ Neutralize LIKE wildcards in user-supplied search text.
+
+ Without this a user typing ``%`` or ``_`` would match every row, which is
+ both wrong and, on a large table, a scan the search was meant to avoid.
+ """
+ return (
+ value.replace(LIKE_ESCAPE_CHAR, LIKE_ESCAPE_CHAR * 2)
+ .replace("%", f"{LIKE_ESCAPE_CHAR}%")
+ .replace("_", f"{LIKE_ESCAPE_CHAR}_")
+ )
+
+
+def build_like_predicate(
+ expr: ColumnElement[Any],
+ search: str,
+) -> ColumnElement[Any]:
+ """
+ Build a case-insensitive containment predicate for ``expr``.
+
+ ``lower(expr) LIKE lower('%term%')`` is used rather than ``ILIKE`` because
+ the latter is not portable across engines.
+ """
+ pattern = f"%{escape_like_pattern(search)}%".lower()
+ return sa.func.lower(expr).like(pattern, escape=LIKE_ESCAPE_CHAR)
+
+
def _normalize_mssql_virtual_dataset_sql(
sql: str, parsed_script: SQLScript, engine: str
) -> str:
@@ -4010,6 +4045,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
limit: int = 10000,
denormalize_column: bool = False,
array_elements: bool = False,
+ search: str | None = None,
) -> list[Any]:
# denormalize column name before querying for values
# unless disabled in the dataset configuration
@@ -4047,6 +4083,9 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
.select_from(tbl)
.distinct()
)
+ if search:
+ qry = qry.where(build_like_predicate(value_expr, search))
+
if limit:
qry = qry.limit(limit)
diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py
index 4f9f7d85dcf..241fc7faed4 100644
--- a/tests/integration_tests/datasource/api_tests.py
+++ b/tests/integration_tests/datasource/api_tests.py
@@ -155,6 +155,7 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=False,
array_elements=False,
+ search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@@ -170,6 +171,79 @@ class TestDatasourceApi(SupersetTestCase):
)
assert values_for_column_mock.call_args.kwargs["array_elements"] is True
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ def test_get_column_values_search_filters_server_side(self):
+ """``?q=`` narrows the values in the database rather than client-side,
+ which is what makes a value beyond the row limit reachable at all."""
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ rv = self.client.get(
+ f"api/v1/datasource/table/{table.id}/column/col2/values/?q=b"
+ )
+ assert rv.status_code == 200
+ assert json.loads(rv.data.decode("utf-8"))["result"] == ["b"]
+
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ def test_get_column_values_search_is_case_insensitive(self):
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ rv = self.client.get(
+ f"api/v1/datasource/table/{table.id}/column/col2/values/?q=B"
+ )
+ assert rv.status_code == 200
+ assert json.loads(rv.data.decode("utf-8"))["result"] == ["b"]
+
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ def test_get_column_values_search_escapes_wildcards(self):
+ """A literal ``%`` must not be treated as "match everything"."""
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ rv = self.client.get(
+ f"api/v1/datasource/table/{table.id}/column/col2/values/?q=%25"
+ )
+ assert rv.status_code == 200
+ assert json.loads(rv.data.decode("utf-8"))["result"] == []
+
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ @patch("superset.models.helpers.ExploreMixin.values_for_column")
+ def test_get_column_values_blank_search_is_ignored(self, values_for_column_mock):
+ """Whitespace is not a search term; it must not narrow the list."""
+ values_for_column_mock.return_value = []
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ self.client.get(
+ f"api/v1/datasource/table/{table.id}/column/col2/values/?q=%20%20"
+ )
+ assert values_for_column_mock.call_args.kwargs["search"] is None
+
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ def test_get_column_values_returns_applied_limit(self):
+ """The client needs the limit to tell a short list from a truncated
+ one, so it can say the list is partial instead of implying it is whole."""
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col2/values/")
+ assert rv.status_code == 200
+ assert json.loads(rv.data.decode("utf-8"))["limit"] == 10000
+
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ @patch("superset.models.helpers.ExploreMixin.values_for_column")
+ def test_get_column_values_cache_isolated_per_search(self, values_for_column_mock):
+ """Search terms must partition the cache; sharing one entry would serve
+ the results of somebody else's search."""
+ cache_manager.data_cache.clear()
+ values_for_column_mock.return_value = ["x"]
+ self.login(ADMIN_USERNAME)
+ table = self.get_virtual_dataset()
+ url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
+
+ self.client.get(url)
+ self.client.get(f"{url}?q=a")
+ self.client.get(f"{url}?q=b")
+ self.client.get(f"{url}?q=a")
+
+ assert values_for_column_mock.call_count == 3
+
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.db_engine_specs.base.BaseEngineSpec.denormalize_name")
def test_get_column_values_not_denormalize_column(self, denormalize_name_mock):
@@ -191,6 +265,7 @@ class TestDatasourceApi(SupersetTestCase):
limit=10000,
denormalize_column=True,
array_elements=False,
+ search=None,
)
@pytest.mark.usefixtures("app_context", "virtual_dataset")
diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py
index ca89ad7e622..47f97f0239c 100644
--- a/tests/unit_tests/models/helpers_test.py
+++ b/tests/unit_tests/models/helpers_test.py
@@ -105,6 +105,86 @@ def test_values_for_column(database: Database) -> None:
assert table.values_for_column("a") == [1, None]
+@pytest.mark.parametrize(
+ "raw,expected",
+ [
+ ("plain", "plain"),
+ ("50%", "50!%"),
+ ("a_b", "a!_b"),
+ ("wow!", "wow!!"),
+ ("!%_", "!!!%!_"),
+ ],
+)
+def test_escape_like_pattern(raw: str, expected: str) -> None:
+ """Wildcards typed by a user are data, not pattern syntax."""
+ from superset.models.helpers import escape_like_pattern
+
+ assert escape_like_pattern(raw) == expected
+
+
+def test_build_like_predicate_is_case_insensitive_and_escaped() -> None:
+ import sqlalchemy as sa
+
+ from superset.models.helpers import build_like_predicate
+
+ compiled = str(
+ build_like_predicate(sa.column("c"), "50%").compile(
+ dialect=sa.dialects.registry.load("postgresql")(),
+ compile_kwargs={"literal_binds": True},
+ )
+ ).replace("%%", "%")
+
+ assert compiled == "lower(c) LIKE '%50!%%' ESCAPE '!'"
+
+
+def test_values_for_column_search(database: Database) -> None:
+ """``search`` narrows the distinct-value query in the database."""
+ import pandas as pd
+
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ database=database,
+ schema=None,
+ table_name="t",
+ columns=[TableColumn(column_name="a")],
+ )
+
+ with patch(
+ "pandas.read_sql_query",
+ return_value=pd.DataFrame({"column_values": ["Alice"]}),
+ ) as read_sql_query:
+ assert table.values_for_column("a", search="ali") == ["Alice"]
+
+ sql = str(read_sql_query.call_args.kwargs["sql"])
+ assert "LIKE" in sql
+ assert "'%ali%'" in sql
+
+
+def test_values_for_column_without_search_has_no_predicate(
+ database: Database,
+) -> None:
+ """The unsearched list must stay a plain bounded DISTINCT scan."""
+ import pandas as pd
+
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ database=database,
+ schema=None,
+ table_name="t",
+ columns=[TableColumn(column_name="a")],
+ )
+
+ with patch(
+ "pandas.read_sql_query",
+ return_value=pd.DataFrame({"column_values": ["Alice"]}),
+ ) as read_sql_query:
+ table.values_for_column("a")
+
+ assert "LIKE" not in str(read_sql_query.call_args.kwargs["sql"])
+
+
def test_values_for_column_passes_catalog_and_schema(
mocker: MockerFixture,
session: Session,