diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts index 603cc7f2c8d7be727e3c04665184ba601a6bb165..319938b95e712930b6793284e7f228279cc3301d 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/table.test.ts @@ -252,4 +252,215 @@ describe('Visualization > Table', () => { }); cy.get('td').contains(/\d*%/); }); + + it('Test row limit with server pagination toggle', () => { + cy.visitChartByParams({ + ...VIZ_DEFAULTS, + metrics: ['count'], + row_limit: 100, + }); + + // Enable server pagination + cy.get('[data-test="server_pagination-header"] div.pull-left').click(); + + // Click row limit control and select high value (200k) + cy.get('div[aria-label="Row limit"]').click(); + + // Type 200000 and press enter to select the option + cy.get('div[aria-label="Row limit"]') + .find('.ant-select-selection-search-input:visible') + .type('200000{enter}'); + + // Verify that there is no error tooltip when server pagination is enabled + cy.get('[data-test="error-tooltip"]').should('not.exist'); + + // Disable server pagination + cy.get('[data-test="server_pagination-header"] div.pull-left').click(); + + // Verify error tooltip appears + cy.get('[data-test="error-tooltip"]').should('be.visible'); + + // Trigger mouseover and verify tooltip text + cy.get('[data-test="error-tooltip"]').trigger('mouseover'); + + // Verify tooltip content + cy.get('.antd5-tooltip-inner').should('be.visible'); + cy.get('.antd5-tooltip-inner').should( + 'contain', + 'Server pagination needs to be enabled for values over', + ); + + // Hide the tooltip by adding display:none style + cy.get('.antd5-tooltip').invoke('attr', 'style', 'display: none'); + + // Enable server pagination again + cy.get('[data-test="server_pagination-header"] div.pull-left').click(); + + cy.get('[data-test="error-tooltip"]').should('not.exist'); + + cy.get('div[aria-label="Row limit"]').click(); + + // Type 1000000 + cy.get('div[aria-label="Row limit"]') + .find('.ant-select-selection-search-input:visible') + .type('1000000'); + + // Wait for 1 second + cy.wait(1000); + + // Press enter + cy.get('div[aria-label="Row limit"]') + .find('.ant-select-selection-search-input:visible') + .type('{enter}'); + + // Wait for error tooltip to appear and verify its content + cy.get('[data-test="error-tooltip"]') + .should('be.visible') + .trigger('mouseover'); + + // Wait for tooltip content and verify + cy.get('.antd5-tooltip-inner').should('exist'); + cy.get('.antd5-tooltip-inner').should('be.visible'); + + // Verify tooltip content separately + cy.get('.antd5-tooltip-inner').should('contain', 'Value cannot exceed'); + }); + + it('Test sorting with server pagination enabled', () => { + cy.visitChartByParams({ + ...VIZ_DEFAULTS, + metrics: ['count'], + groupby: ['name'], + row_limit: 100000, + server_pagination: true, // Enable server pagination + }); + + // Wait for the initial data load + cy.wait('@chartData'); + + // Get the first column header (name) + cy.get('.chart-container th').contains('name').as('nameHeader'); + + // Click to sort ascending + cy.get('@nameHeader').click(); + cy.wait('@chartData'); + + // Verify first row starts with 'A' + cy.get('.chart-container td:first').invoke('text').should('match', /^[Aa]/); + + // Click again to sort descending + cy.get('@nameHeader').click(); + cy.wait('@chartData'); + + // Verify first row starts with 'Z' + cy.get('.chart-container td:first').invoke('text').should('match', /^[Zz]/); + + // Test numeric sorting + cy.get('.chart-container th').contains('COUNT').as('countHeader'); + + // Click to sort ascending by count + cy.get('@countHeader').click(); + cy.wait('@chartData'); + + // Get first two count values and verify ascending order + cy.get('.chart-container td:nth-child(2)').then($cells => { + const first = parseFloat($cells[0].textContent || '0'); + const second = parseFloat($cells[1].textContent || '0'); + expect(first).to.be.at.most(second); + }); + + // Click again to sort descending + cy.get('@countHeader').click(); + cy.wait('@chartData'); + + // Get first two count values and verify descending order + cy.get('.chart-container td:nth-child(2)').then($cells => { + const first = parseFloat($cells[0].textContent || '0'); + const second = parseFloat($cells[1].textContent || '0'); + expect(first).to.be.at.least(second); + }); + }); + + it('Test search with server pagination enabled', () => { + cy.visitChartByParams({ + ...VIZ_DEFAULTS, + metrics: ['count'], + groupby: ['name', 'state'], + row_limit: 100000, + server_pagination: true, + include_search: true, + }); + + cy.wait('@chartData'); + + // Basic search test + cy.get('span.dt-global-filter input.form-control.input-sm').should( + 'be.visible', + ); + + cy.get('span.dt-global-filter input.form-control.input-sm').type('John'); + + cy.wait('@chartData'); + + cy.get('.chart-container tbody tr').each($row => { + cy.wrap($row).contains(/John/i); + }); + + // Clear and test case-insensitive search + cy.get('span.dt-global-filter input.form-control.input-sm').clear(); + + cy.wait('@chartData'); + + cy.get('span.dt-global-filter input.form-control.input-sm').type('mary'); + + cy.wait('@chartData'); + + cy.get('.chart-container tbody tr').each($row => { + cy.wrap($row).contains(/Mary/i); + }); + + // Test special characters + cy.get('span.dt-global-filter input.form-control.input-sm').clear(); + + cy.get('span.dt-global-filter input.form-control.input-sm').type('Nicole'); + + cy.wait('@chartData'); + + cy.get('.chart-container tbody tr').each($row => { + cy.wrap($row).contains(/Nicole/i); + }); + + // Test no results + cy.get('span.dt-global-filter input.form-control.input-sm').clear(); + + cy.get('span.dt-global-filter input.form-control.input-sm').type('XYZ123'); + + cy.wait('@chartData'); + + cy.get('.chart-container').contains('No records found'); + + // Test column-specific search + cy.get('.search-select').should('be.visible'); + + cy.get('.search-select').click(); + + cy.get('.ant-select-dropdown').should('be.visible'); + + cy.get('.ant-select-item-option').contains('state').should('be.visible'); + + cy.get('.ant-select-item-option').contains('state').click(); + + cy.get('span.dt-global-filter input.form-control.input-sm').clear(); + + cy.get('span.dt-global-filter input.form-control.input-sm').type('CA'); + + cy.wait('@chartData'); + cy.wait(1000); + + cy.get('td[aria-labelledby="header-state"]').should('be.visible'); + + cy.get('td[aria-labelledby="header-state"]') + .first() + .should('contain', 'CA'); + }); }); diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts index c9e8bf9db7fd2c32ff30ddf9f6ab4174f2cd3935..394c2c7f309c1e8d648cbd4309bebe9cade9e774 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts @@ -26,6 +26,7 @@ import { import { ColumnMeta, SortSeriesData, SortSeriesType } from './types'; export const DEFAULT_MAX_ROW = 100000; +export const DEFAULT_MAX_ROW_TABLE_SERVER = 500000; // eslint-disable-next-line import/prefer-default-export export const TIME_FILTER_LABELS = { diff --git a/superset-frontend/packages/superset-ui-core/src/validator/index.ts b/superset-frontend/packages/superset-ui-core/src/validator/index.ts index 1198c4e0a5b4d859c292bd069f725a3b959294d9..df8dc10a70f973727195d2266f9e210db7ad10ae 100644 --- a/superset-frontend/packages/superset-ui-core/src/validator/index.ts +++ b/superset-frontend/packages/superset-ui-core/src/validator/index.ts @@ -25,3 +25,4 @@ export { default as validateNonEmpty } from './validateNonEmpty'; export { default as validateMaxValue } from './validateMaxValue'; export { default as validateMapboxStylesUrl } from './validateMapboxStylesUrl'; export { default as validateTimeComparisonRangeValues } from './validateTimeComparisonRangeValues'; +export { default as validateServerPagination } from './validateServerPagination'; diff --git a/superset-frontend/packages/superset-ui-core/src/validator/validateServerPagination.ts b/superset-frontend/packages/superset-ui-core/src/validator/validateServerPagination.ts new file mode 100644 index 0000000000000000000000000000000000000000..02db3585a1ac9c4cd93ff135013638f8c4410739 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/validator/validateServerPagination.ts @@ -0,0 +1,30 @@ +/** + * 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 { t } from '../translation'; + +export default function validateServerPagination( + v: unknown, + serverPagination: boolean, + max: number, +) { + if (Number(v) > +max && !serverPagination) { + return t('Server pagination needs to be enabled for values over %s', max); + } + return false; +} diff --git a/superset-frontend/packages/superset-ui-core/test/validator/validateServerPagination.test.ts b/superset-frontend/packages/superset-ui-core/test/validator/validateServerPagination.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..5a638e5142f4b974b2914bc2e48bae51b6c8e2c0 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/test/validator/validateServerPagination.test.ts @@ -0,0 +1,46 @@ +/** + * 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 { validateServerPagination } from '@superset-ui/core'; +import './setup'; + +test('validateServerPagination returns warning message when server pagination is disabled and value exceeds max', () => { + expect(validateServerPagination(100001, false, 100000)).toBeTruthy(); + expect(validateServerPagination('150000', false, 100000)).toBeTruthy(); + expect(validateServerPagination(200000, false, 100000)).toBeTruthy(); +}); + +test('validateServerPagination returns false when server pagination is enabled', () => { + expect(validateServerPagination(100001, true, 100000)).toBeFalsy(); + expect(validateServerPagination(150000, true, 100000)).toBeFalsy(); + expect(validateServerPagination('200000', true, 100000)).toBeFalsy(); +}); + +test('validateServerPagination returns false when value is below max', () => { + expect(validateServerPagination(50000, false, 100000)).toBeFalsy(); + expect(validateServerPagination('75000', false, 100000)).toBeFalsy(); + expect(validateServerPagination(99999, false, 100000)).toBeFalsy(); +}); + +test('validateServerPagination handles edge cases', () => { + expect(validateServerPagination(undefined, false, 100000)).toBeFalsy(); + expect(validateServerPagination(null, false, 100000)).toBeFalsy(); + expect(validateServerPagination(NaN, false, 100000)).toBeFalsy(); + expect(validateServerPagination('invalid', false, 100000)).toBeFalsy(); +}); diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index 21ce6541cf2fdb765ed7eae773d920053af37208..3aacefb2f300cf02ce768db067b6c7ca4d781238 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +/* eslint-disable import/no-extraneous-dependencies */ import { useCallback, useRef, @@ -24,6 +25,7 @@ import { MutableRefObject, CSSProperties, DragEvent, + useEffect, } from 'react'; import { @@ -39,8 +41,9 @@ import { Row, } from 'react-table'; import { matchSorter, rankings } from 'match-sorter'; -import { typedMemo, usePrevious } from '@superset-ui/core'; +import { styled, typedMemo, usePrevious } from '@superset-ui/core'; import { isEqual } from 'lodash'; +import { Space } from 'antd'; import GlobalFilter, { GlobalFilterProps } from './components/GlobalFilter'; import SelectPageSize, { SelectPageSizeProps, @@ -50,6 +53,8 @@ import SimplePagination from './components/Pagination'; import useSticky from './hooks/useSticky'; import { PAGE_SIZE_OPTIONS } from '../consts'; import { sortAlphanumericCaseInsensitive } from './utils/sortAlphanumericCaseInsensitive'; +import { SearchOption, SortByItem } from '../types'; +import SearchSelectDropdown from './components/SearchSelectDropdown'; export interface DataTableProps<D extends object> extends TableOptions<D> { tableClassName?: string; @@ -62,7 +67,12 @@ export interface DataTableProps<D extends object> extends TableOptions<D> { height?: string | number; serverPagination?: boolean; onServerPaginationChange: (pageNumber: number, pageSize: number) => void; - serverPaginationData: { pageSize?: number; currentPage?: number }; + serverPaginationData: { + pageSize?: number; + currentPage?: number; + sortBy?: SortByItem[]; + searchColumn?: string; + }; pageSize?: number; noResults?: string | ((filterString: string) => ReactNode); sticky?: boolean; @@ -71,6 +81,14 @@ export interface DataTableProps<D extends object> extends TableOptions<D> { onColumnOrderChange: () => void; renderGroupingHeaders?: () => JSX.Element; renderTimeComparisonDropdown?: () => JSX.Element; + handleSortByChange: (sortBy: SortByItem[]) => void; + sortByFromParent: SortByItem[]; + manualSearch?: boolean; + onSearchChange?: (searchText: string) => void; + initialSearchText?: string; + searchInputId?: string; + onSearchColChange: (searchCol: string) => void; + searchOptions: SearchOption[]; } export interface RenderHTMLCellProps extends HTMLProps<HTMLTableCellElement> { @@ -81,6 +99,20 @@ const sortTypes = { alphanumeric: sortAlphanumericCaseInsensitive, }; +const StyledSpace = styled(Space)` + display: flex; + justify-content: flex-end; + + .search-select-container { + display: flex; + } + + .search-by-label { + align-self: center; + margin-right: 4px; + } +`; + // Be sure to pass our updateMyData and the skipReset option export default typedMemo(function DataTable<D extends object>({ tableClassName, @@ -105,6 +137,14 @@ export default typedMemo(function DataTable<D extends object>({ onColumnOrderChange, renderGroupingHeaders, renderTimeComparisonDropdown, + handleSortByChange, + sortByFromParent = [], + manualSearch = false, + onSearchChange, + initialSearchText, + searchInputId, + onSearchColChange, + searchOptions, ...moreUseTableOptions }: DataTableProps<D>): JSX.Element { const tableHooks: PluginHook<D>[] = [ @@ -115,6 +155,7 @@ export default typedMemo(function DataTable<D extends object>({ doSticky ? useSticky : [], hooks || [], ].flat(); + const columnNames = Object.keys(data?.[0] || {}); const previousColumnNames = usePrevious(columnNames); const resultsSize = serverPagination ? rowCount : data.length; @@ -127,7 +168,8 @@ export default typedMemo(function DataTable<D extends object>({ ...initialState_, // zero length means all pages, the `usePagination` plugin does not // understand pageSize = 0 - sortBy: sortByRef.current, + // sortBy: sortByRef.current, + sortBy: serverPagination ? sortByFromParent : sortByRef.current, pageSize: initialPageSize > 0 ? initialPageSize : resultsSize || 10, }; const defaultWrapperRef = useRef<HTMLDivElement>(null); @@ -188,7 +230,13 @@ export default typedMemo(function DataTable<D extends object>({ wrapStickyTable, setColumnOrder, allColumns, - state: { pageIndex, pageSize, globalFilter: filterValue, sticky = {} }, + state: { + pageIndex, + pageSize, + globalFilter: filterValue, + sticky = {}, + sortBy, + }, } = useTable<D>( { columns, @@ -198,10 +246,46 @@ export default typedMemo(function DataTable<D extends object>({ globalFilter: defaultGlobalFilter, sortTypes, autoResetSortBy: !isEqual(columnNames, previousColumnNames), + manualSortBy: !!serverPagination, ...moreUseTableOptions, }, ...tableHooks, ); + + const handleSearchChange = useCallback( + (query: string) => { + if (manualSearch && onSearchChange) { + onSearchChange(query); + } else { + setGlobalFilter(query); + } + }, + [manualSearch, onSearchChange, setGlobalFilter], + ); + + // updating the sort by to the own State of table viz + useEffect(() => { + const serverSortBy = serverPaginationData?.sortBy || []; + + if (serverPagination && !isEqual(sortBy, serverSortBy)) { + if (Array.isArray(sortBy) && sortBy.length > 0) { + const [sortByItem] = sortBy; + const matchingColumn = columns.find(col => col?.id === sortByItem?.id); + + if (matchingColumn && 'columnKey' in matchingColumn) { + const sortByWithColumnKey: SortByItem = { + ...sortByItem, + key: (matchingColumn as { columnKey: string }).columnKey, + }; + + handleSortByChange([sortByWithColumnKey]); + } + } else { + handleSortByChange([]); + } + } + }, [sortBy]); + // make setPageSize accept 0 const setPageSize = (size: number) => { if (serverPagination) { @@ -355,6 +439,7 @@ export default typedMemo(function DataTable<D extends object>({ resultOnPageChange = (pageNumber: number) => onServerPaginationChange(pageNumber, serverPageSize); } + return ( <div ref={wrapperRef} @@ -381,16 +466,31 @@ export default typedMemo(function DataTable<D extends object>({ ) : null} </div> {searchInput ? ( - <div className="col-sm-6"> + <StyledSpace className="col-sm-6"> + {serverPagination && ( + <div className="search-select-container"> + <span className="search-by-label">Search by: </span> + <SearchSelectDropdown + searchOptions={searchOptions} + value={serverPaginationData?.searchColumn || ''} + onChange={onSearchColChange} + /> + </div> + )} <GlobalFilter<D> searchInput={ typeof searchInput === 'boolean' ? undefined : searchInput } preGlobalFilteredRows={preGlobalFilteredRows} - setGlobalFilter={setGlobalFilter} - filterValue={filterValue} + setGlobalFilter={ + manualSearch ? handleSearchChange : setGlobalFilter + } + filterValue={manualSearch ? initialSearchText : filterValue} + id={searchInputId} + serverPagination={!!serverPagination} + rowCount={rowCount} /> - </div> + </StyledSpace> ) : null} {renderTimeComparisonDropdown ? ( <div diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/GlobalFilter.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/GlobalFilter.tsx index 89e18140f22eddb6ffc4727ec5e515455fa4fa0c..75d6594cbcd446c52b6aa52377454e4c6d7b20f2 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/GlobalFilter.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/GlobalFilter.tsx @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { memo, ComponentType, ChangeEventHandler } from 'react'; +import { + memo, + ComponentType, + ChangeEventHandler, + useRef, + useEffect, +} from 'react'; import { Row, FilterValue } from 'react-table'; import useAsyncState from '../utils/useAsyncState'; @@ -24,8 +30,12 @@ export interface SearchInputProps { count: number; value: string; onChange: ChangeEventHandler<HTMLInputElement>; + onBlur?: () => void; + inputRef?: React.RefObject<HTMLInputElement>; } +const isSearchFocused = new Map(); + export interface GlobalFilterProps<D extends object> { preGlobalFilteredRows: Row<D>[]; // filter value cannot be `undefined` otherwise React will report component @@ -33,17 +43,28 @@ export interface GlobalFilterProps<D extends object> { filterValue: string; setGlobalFilter: (filterValue: FilterValue) => void; searchInput?: ComponentType<SearchInputProps>; + id?: string; + serverPagination: boolean; + rowCount: number; } -function DefaultSearchInput({ count, value, onChange }: SearchInputProps) { +function DefaultSearchInput({ + count, + value, + onChange, + onBlur, + inputRef, +}: SearchInputProps) { return ( <span className="dt-global-filter"> Search{' '} <input + ref={inputRef} className="form-control input-sm" placeholder={`${count} records...`} value={value} onChange={onChange} + onBlur={onBlur} /> </span> ); @@ -56,8 +77,13 @@ export default (memo as <T>(fn: T) => T)(function GlobalFilter< filterValue = '', searchInput, setGlobalFilter, + id = '', + serverPagination, + rowCount, }: GlobalFilterProps<D>) { - const count = preGlobalFilteredRows.length; + const count = serverPagination ? rowCount : preGlobalFilteredRows.length; + const inputRef = useRef<HTMLInputElement>(null); + const [value, setValue] = useAsyncState( filterValue, (newValue: string) => { @@ -66,17 +92,37 @@ export default (memo as <T>(fn: T) => T)(function GlobalFilter< 200, ); + // Preserve focus during server-side filtering to maintain a better user experience + useEffect(() => { + if ( + serverPagination && + isSearchFocused.get(id) && + document.activeElement !== inputRef.current + ) { + inputRef.current?.focus(); + } + }, [value, serverPagination]); + + const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const target = e.target as HTMLInputElement; + e.preventDefault(); + isSearchFocused.set(id, true); + setValue(target.value); + }; + + const handleBlur = () => { + isSearchFocused.set(id, false); + }; + const SearchInput = searchInput || DefaultSearchInput; return ( <SearchInput count={count} value={value} - onChange={e => { - const target = e.target as HTMLInputElement; - e.preventDefault(); - setValue(target.value); - }} + inputRef={inputRef} + onChange={handleChange} + onBlur={handleBlur} /> ); }); diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/SearchSelectDropdown.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/SearchSelectDropdown.tsx new file mode 100644 index 0000000000000000000000000000000000000000..b58cf2e8a2259d8f565169a1b876fb5bf381fe16 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/components/SearchSelectDropdown.tsx @@ -0,0 +1,53 @@ +/** + * 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. + */ +/* eslint-disable import/no-extraneous-dependencies */ +import { styled } from '@superset-ui/core'; +import { Select } from 'antd'; +import { SearchOption } from '../../types'; + +const StyledSelect = styled(Select)` + width: 120px; + margin-right: 8px; +`; + +interface SearchSelectDropdownProps { + /** The currently selected search column value */ + value?: string; + /** Callback triggered when a new search column is selected */ + onChange: (searchCol: string) => void; + /** Available search column options to populate the dropdown */ + searchOptions: SearchOption[]; +} + +function SearchSelectDropdown({ + value, + onChange, + searchOptions, +}: SearchSelectDropdownProps) { + return ( + <StyledSelect + className="search-select" + value={value || (searchOptions?.[0]?.value ?? '')} + options={searchOptions} + onChange={onChange} + /> + ); +} + +export default SearchSelectDropdown; diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts b/superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts index 6c319f768453cfc7781da5f836ac450d0414b31b..9d591a0818260ca376664fef6c82e32e50789f59 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts @@ -115,3 +115,11 @@ declare module 'react-table' { extends UseTableHooks<D>, UseSortByHooks<D> {} } + +interface TableOwnState { + currentPage?: number; + pageSize?: number; + sortColumn?: string; + sortOrder?: 'asc' | 'desc'; + searchText?: string; +} diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts b/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts index b703d66d97153997152e6f4208ecf2b2ed75b1a6..8a5d20ed5bca9844066d43a787249fc3c0457d15 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts @@ -18,6 +18,7 @@ */ import { SetDataMaskHook } from '@superset-ui/core'; +import { TableOwnState } from '../types/react-table'; export const updateExternalFormData = ( setDataMask: SetDataMaskHook = () => {}, @@ -30,3 +31,11 @@ export const updateExternalFormData = ( pageSize, }, }); + +export const updateTableOwnState = ( + setDataMask: SetDataMaskHook = () => {}, + modifiedOwnState: TableOwnState, +) => + setDataMask({ + ownState: modifiedOwnState, + }); diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 88a326544e6c4640259e47b2e6274a04cfbdae79..475056c75034759661f9c93889a253e649737981 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -24,6 +24,7 @@ import { useState, MouseEvent, KeyboardEvent as ReactKeyboardEvent, + useEffect, } from 'react'; import { @@ -61,10 +62,12 @@ import { PlusCircleOutlined, TableOutlined, } from '@ant-design/icons'; -import { isEmpty } from 'lodash'; +import { debounce, isEmpty, isEqual } from 'lodash'; import { ColorSchemeEnum, DataColumnMeta, + SearchOption, + SortByItem, TableChartTransformedProps, } from './types'; import DataTable, { @@ -77,7 +80,7 @@ import DataTable, { import Styles from './Styles'; import { formatColumnValue } from './utils/formatValue'; import { PAGE_SIZE_OPTIONS } from './consts'; -import { updateExternalFormData } from './DataTable/utils/externalAPIs'; +import { updateTableOwnState } from './DataTable/utils/externalAPIs'; import getScrollBarSize from './DataTable/utils/getScrollBarSize'; type ValueRange = [number, number]; @@ -176,20 +179,26 @@ function SortIcon<D extends object>({ column }: { column: ColumnInstance<D> }) { return sortIcon; } -function SearchInput({ count, value, onChange }: SearchInputProps) { - return ( - <span className="dt-global-filter"> - {t('Search')}{' '} - <input - aria-label={t('Search %s records', count)} - className="form-control input-sm" - placeholder={tn('search.num_records', count)} - value={value} - onChange={onChange} - /> - </span> - ); -} +const SearchInput = ({ + count, + value, + onChange, + onBlur, + inputRef, +}: SearchInputProps) => ( + <span className="dt-global-filter"> + {t('Search')}{' '} + <input + ref={inputRef} + aria-label={t('Search %s records', count)} + className="form-control input-sm" + placeholder={tn('search.num_records', count)} + value={value} + onChange={onChange} + onBlur={onBlur} + /> + </span> +); function SelectPageSize({ options, @@ -267,6 +276,9 @@ export default function TableChart<D extends DataRecord = DataRecord>( isUsingTimeComparison, basicColorFormatters, basicColorColumnFormatters, + hasServerPageLengthChanged, + serverPageLength, + slice_id, } = props; const comparisonColumns = [ { key: 'all', label: t('Display all') }, @@ -679,7 +691,12 @@ export default function TableChart<D extends DataRecord = DataRecord>( ); const getColumnConfigs = useCallback( - (column: DataColumnMeta, i: number): ColumnWithLooseAccessor<D> => { + ( + column: DataColumnMeta, + i: number, + ): ColumnWithLooseAccessor<D> & { + columnKey: string; + } => { const { key, label: originalLabel, @@ -766,6 +783,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( // must use custom accessor to allow `.` in column names // typing is incorrect in current version of `@types/react-table` // so we ask TS not to check. + columnKey: key, accessor: ((datum: D) => datum[key]) as never, Cell: ({ value, row }: { value: DataRecordValue; row: Row<D> }) => { const [isHtml, text] = formatColumnValue(column, value); @@ -1058,13 +1076,50 @@ export default function TableChart<D extends DataRecord = DataRecord>( [visibleColumnsMeta, getColumnConfigs], ); + const [searchOptions, setSearchOptions] = useState<SearchOption[]>([]); + + useEffect(() => { + const options = ( + columns as unknown as ColumnWithLooseAccessor & + { + columnKey: string; + sortType?: string; + }[] + ) + .filter(col => col?.sortType === 'alphanumeric') + .map(column => ({ + value: column.columnKey, + label: column.columnKey, + })); + + if (!isEqual(options, searchOptions)) { + setSearchOptions(options || []); + } + }, [columns]); + const handleServerPaginationChange = useCallback( (pageNumber: number, pageSize: number) => { - updateExternalFormData(setDataMask, pageNumber, pageSize); + const modifiedOwnState = { + ...serverPaginationData, + currentPage: pageNumber, + pageSize, + }; + updateTableOwnState(setDataMask, modifiedOwnState); }, [setDataMask], ); + useEffect(() => { + if (hasServerPageLengthChanged) { + const modifiedOwnState = { + ...serverPaginationData, + currentPage: 0, + pageSize: serverPageLength, + }; + updateTableOwnState(setDataMask, modifiedOwnState); + } + }, []); + const handleSizeChange = useCallback( ({ width, height }: { width: number; height: number }) => { setTableSize({ width, height }); @@ -1100,6 +1155,42 @@ export default function TableChart<D extends DataRecord = DataRecord>( const { width: widthFromState, height: heightFromState } = tableSize; + const handleSortByChange = useCallback( + (sortBy: SortByItem[]) => { + if (!serverPagination) return; + const modifiedOwnState = { + ...serverPaginationData, + sortBy, + }; + updateTableOwnState(setDataMask, modifiedOwnState); + }, + [setDataMask, serverPagination], + ); + + const handleSearch = (searchText: string) => { + const modifiedOwnState = { + ...(serverPaginationData || {}), + searchColumn: + serverPaginationData?.searchColumn || searchOptions[0]?.value, + searchText, + currentPage: 0, // Reset to first page when searching + }; + updateTableOwnState(setDataMask, modifiedOwnState); + }; + + const debouncedSearch = debounce(handleSearch, 800); + + const handleChangeSearchCol = (searchCol: string) => { + if (!isEqual(searchCol, serverPaginationData?.searchColumn)) { + const modifiedOwnState = { + ...(serverPaginationData || {}), + searchColumn: searchCol, + searchText: '', + }; + updateTableOwnState(setDataMask, modifiedOwnState); + } + }; + return ( <Styles> <DataTable<D> @@ -1115,6 +1206,9 @@ export default function TableChart<D extends DataRecord = DataRecord>( serverPagination={serverPagination} onServerPaginationChange={handleServerPaginationChange} onColumnOrderChange={() => setColumnOrderToggle(!columnOrderToggle)} + initialSearchText={serverPaginationData?.searchText || ''} + sortByFromParent={serverPaginationData?.sortBy || []} + searchInputId={`${slice_id}-search`} // 9 page items in > 340px works well even for 100+ pages maxPageItemCount={width > 340 ? 9 : 7} noResults={getNoResultsMessage} @@ -1128,6 +1222,11 @@ export default function TableChart<D extends DataRecord = DataRecord>( renderTimeComparisonDropdown={ isUsingTimeComparison ? renderTimeComparisonDropdown : undefined } + handleSortByChange={handleSortByChange} + onSearchColChange={handleChangeSearchCol} + manualSearch={serverPagination} + onSearchChange={debouncedSearch} + searchOptions={searchOptions} /> </Styles> ); diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 5b9cb684ed4a4474737834b73e26dc52a5caa536..439df369f5a8d736e59a404dd02271f50c9a2bcb 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -22,6 +22,7 @@ import { ensureIsArray, getMetricLabel, isPhysicalColumn, + QueryFormOrderBy, QueryMode, QueryObject, removeDuplicates, @@ -34,7 +35,7 @@ import { } from '@superset-ui/chart-controls'; import { isEmpty } from 'lodash'; import { TableChartFormData } from './types'; -import { updateExternalFormData } from './DataTable/utils/externalAPIs'; +import { updateTableOwnState } from './DataTable/utils/externalAPIs'; /** * Infer query mode from form data. If `all_columns` is set, then raw records mode, @@ -191,18 +192,40 @@ const buildQuery: BuildQuery<TableChartFormData> = ( const moreProps: Partial<QueryObject> = {}; const ownState = options?.ownState ?? {}; - if (formDataCopy.server_pagination) { - moreProps.row_limit = - ownState.pageSize ?? formDataCopy.server_page_length; - moreProps.row_offset = - (ownState.currentPage ?? 0) * (ownState.pageSize ?? 0); + // Build Query flag to check if its for either download as csv, excel or json + const isDownloadQuery = + ['csv', 'xlsx'].includes(formData?.result_format || '') || + (formData?.result_format === 'json' && + formData?.result_type === 'results'); + + if (isDownloadQuery) { + moreProps.row_limit = Number(formDataCopy.row_limit) || 0; + moreProps.row_offset = 0; + } + + if (!isDownloadQuery && formDataCopy.server_pagination) { + const pageSize = ownState.pageSize ?? formDataCopy.server_page_length; + const currentPage = ownState.currentPage ?? 0; + + moreProps.row_limit = pageSize; + moreProps.row_offset = currentPage * pageSize; + } + + // getting sort by in case of server pagination from own state + let sortByFromOwnState: QueryFormOrderBy[] | undefined; + if (Array.isArray(ownState?.sortBy) && ownState?.sortBy.length > 0) { + const sortByItem = ownState?.sortBy[0]; + sortByFromOwnState = [[sortByItem?.key, !sortByItem?.desc]]; } let queryObject = { ...baseQueryObject, columns, extras, - orderby, + orderby: + formData.server_pagination && sortByFromOwnState + ? sortByFromOwnState + : orderby, metrics, post_processing: postProcessing, time_offsets: timeOffsets, @@ -216,11 +239,12 @@ const buildQuery: BuildQuery<TableChartFormData> = ( JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; - updateExternalFormData( - options?.hooks?.setDataMask, - 0, - queryObject.row_limit ?? 0, - ); + const modifiedOwnState = { + ...(options?.ownState || {}), + currentPage: 0, + pageSize: queryObject.row_limit ?? 0, + }; + updateTableOwnState(options?.hooks?.setDataMask, modifiedOwnState); } // Because we use same buildQuery for all table on the page we need split them by id options?.hooks?.setCachedChanges({ @@ -252,12 +276,32 @@ const buildQuery: BuildQuery<TableChartFormData> = ( } if (formData.server_pagination) { + // Add search filter if search text exists + if (ownState.searchText && ownState?.searchColumn) { + queryObject = { + ...queryObject, + filters: [ + ...(queryObject.filters || []), + { + col: ownState?.searchColumn, + op: 'ILIKE', + val: `${ownState.searchText}%`, + }, + ], + }; + } + } + + // Now since row limit control is always visible even + // in case of server pagination + // we must use row limit from form data + if (formData.server_pagination && !isDownloadQuery) { return [ { ...queryObject }, { ...queryObject, time_offsets: [], - row_limit: 0, + row_limit: Number(formData?.row_limit) ?? 0, row_offset: 0, post_processing: [], is_rowcount: true, diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index f6798b798483bf5b38a564342ff7d123bf05d1ae..aab66f800ddb9b556b7b3a52b917e4bac8f937a5 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -28,7 +28,10 @@ import { ControlStateMapping, D3_TIME_FORMAT_OPTIONS, Dataset, + DEFAULT_MAX_ROW, + DEFAULT_MAX_ROW_TABLE_SERVER, defineSavedMetrics, + formatSelectOptions, getStandardizedControls, QueryModeLabel, sections, @@ -40,11 +43,14 @@ import { getMetricLabel, isAdhocColumn, isPhysicalColumn, + legacyValidateInteger, QueryFormColumn, QueryFormMetric, QueryMode, SMART_DATE_ID, t, + validateMaxValue, + validateServerPagination, } from '@superset-ui/core'; import { isEmpty, last } from 'lodash'; @@ -188,6 +194,15 @@ const processComparisonColumns = (columns: any[], suffix: string) => }) .flat(); +/* +Options for row limit control +*/ + +export const ROW_LIMIT_OPTIONS_TABLE = [ + 10, 50, 100, 250, 500, 1000, 5000, 10000, 50000, 100000, 150000, 200000, + 250000, 300000, 350000, 400000, 450000, 500000, +]; + const config: ControlPanelConfig = { controlPanelSections: [ { @@ -342,14 +357,6 @@ const config: ControlPanelConfig = { }, ], [ - { - name: 'row_limit', - override: { - default: 1000, - visibility: ({ controls }: ControlPanelsContainerProps) => - !controls?.server_pagination?.value, - }, - }, { name: 'server_page_length', config: { @@ -364,6 +371,47 @@ const config: ControlPanelConfig = { }, }, ], + [ + { + name: 'row_limit', + config: { + type: 'SelectControl', + freeForm: true, + label: t('Row limit'), + clearable: false, + mapStateToProps: state => ({ + maxValue: state?.common?.conf?.TABLE_VIZ_MAX_ROW_SERVER, + server_pagination: state?.form_data?.server_pagination, + maxValueWithoutServerPagination: + state?.common?.conf?.SQL_MAX_ROW, + }), + validators: [ + legacyValidateInteger, + (v, state) => + validateMaxValue( + v, + state?.maxValue || DEFAULT_MAX_ROW_TABLE_SERVER, + ), + (v, state) => + validateServerPagination( + v, + state?.server_pagination, + state?.maxValueWithoutServerPagination || DEFAULT_MAX_ROW, + ), + ], + // Re run the validations when this control value + validationDependancies: ['server_pagination'], + default: 10000, + choices: formatSelectOptions(ROW_LIMIT_OPTIONS_TABLE), + description: t( + 'Limits the number of the rows that are computed in the query that is the source of the data used for this chart.', + ), + }, + override: { + default: 1000, + }, + }, + ], [ { name: 'order_desc', diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts index d62d9cb92c95b121e16020f9f1bd3f3fe3be440e..86d4efed8f4e7a2332a7399a03ed8361b871d32d 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts @@ -90,6 +90,15 @@ const processDataRecords = memoizeOne(function processDataRecords( return data; }); +// Create a map to store cached values per slice +const sliceCache = new Map< + number, + { + cachedServerLength: number; + passedColumns?: DataColumnMeta[]; + } +>(); + const calculateDifferences = ( originalValue: number, comparisonValue: number, @@ -480,6 +489,7 @@ const transformProps = ( comparison_color_enabled: comparisonColorEnabled = false, comparison_color_scheme: comparisonColorScheme = ColorSchemeEnum.Green, comparison_type, + slice_id, } = formData; const isUsingTimeComparison = !isEmpty(time_compare) && @@ -675,6 +685,26 @@ const transformProps = ( conditionalFormatting, ); + // Get cached values for this slice + const cachedValues = sliceCache.get(slice_id); + let hasServerPageLengthChanged = false; + + if ( + cachedValues?.cachedServerLength !== undefined && + cachedValues.cachedServerLength !== serverPageLength + ) { + hasServerPageLengthChanged = true; + } + + // Update cache with new values + sliceCache.set(slice_id, { + cachedServerLength: serverPageLength, + passedColumns: + Array.isArray(passedColumns) && passedColumns?.length > 0 + ? passedColumns + : cachedValues?.passedColumns, + }); + const startDateOffset = chartProps.rawFormData?.start_date_offset; return { height, @@ -682,7 +712,10 @@ const transformProps = ( isRawRecords: queryMode === QueryMode.Raw, data: passedData, totals, - columns: passedColumns, + columns: + Array.isArray(passedColumns) && passedColumns?.length > 0 + ? passedColumns + : cachedValues?.passedColumns || [], serverPagination, metrics, percentMetrics, @@ -697,7 +730,9 @@ const transformProps = ( includeSearch, rowCount, pageSize: serverPagination - ? serverPageLength + ? serverPaginationData?.pageSize + ? serverPaginationData?.pageSize + : serverPageLength : getPageSize(pageLength, data.length, columns.length), filters: filterState.filters, emitCrossFilters, @@ -711,6 +746,9 @@ const transformProps = ( basicColorFormatters, startDateOffset, basicColorColumnFormatters, + hasServerPageLengthChanged, + serverPageLength, + slice_id, }; }; diff --git a/superset-frontend/plugins/plugin-chart-table/src/types.ts b/superset-frontend/plugins/plugin-chart-table/src/types.ts index 7460a27c461816ff0b2d50f64a7ddb86cc877d4a..1b7cfd45904c4c27ce72d82f5b93c5a915e3fa5c 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/types.ts @@ -114,13 +114,32 @@ export type BasicColorFormatterType = { mainArrow: string; }; +export type SortByItem = { + id: string; + key: string; + desc?: boolean; +}; + +export type SearchOption = { + value: string; + label: string; +}; + +export interface ServerPaginationData { + pageSize?: number; + currentPage?: number; + sortBy?: SortByItem[]; + searchText?: string; + searchColumn?: string; +} + export interface TableChartTransformedProps<D extends DataRecord = DataRecord> { timeGrain?: TimeGranularity; height: number; width: number; rowCount?: number; serverPagination: boolean; - serverPaginationData: { pageSize?: number; currentPage?: number }; + serverPaginationData: ServerPaginationData; setDataMask: SetDataMaskHook; isRawRecords?: boolean; data: D[]; @@ -152,6 +171,11 @@ export interface TableChartTransformedProps<D extends DataRecord = DataRecord> { basicColorFormatters?: { [Key: string]: BasicColorFormatterType }[]; basicColorColumnFormatters?: { [Key: string]: BasicColorFormatterType }[]; startDateOffset?: string; + // For explore page to reset the server Pagination data + // if server page length is changed from control panel + hasServerPageLengthChanged: boolean; + serverPageLength: number; + slice_id: number; } export enum ColorSchemeEnum { diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx index 25fc045d21054d7b40799b241af2d7911557ca23..7b641d0eaecce5c157697ab1664a896694a66a36 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.jsx @@ -327,6 +327,10 @@ class ChartRenderer extends Component { ?.behaviors.find(behavior => behavior === Behavior.DrillToDetail) ? { inContextMenu: this.state.inContextMenu } : {}; + // By pass no result component when server pagination is enabled & the table has a backend search query + const bypassNoResult = !( + formData?.server_pagination && (ownState?.searchText?.length || 0) > 0 + ); return ( <> @@ -367,6 +371,7 @@ class ChartRenderer extends Component { postTransformProps={postTransformProps} emitCrossFilters={emitCrossFilters} legendState={this.state.legendState} + enableNoResults={bypassNoResult} {...drillToDetailProps} /> </div> diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 2677c1e3ff55816409bc7a0d41ab4b032ec6ea27..156459353e82c99b171367cec8a5229c402ea460 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -214,6 +214,52 @@ export default function exploreReducer(state = {}, action) { currentControlsState = transformed.controlsState; } + const dependantControls = Object.entries(state.controls) + .filter( + ([, item]) => + Array.isArray(item?.validationDependancies) && + item.validationDependancies.includes(controlName), + ) + .map(([key, item]) => ({ + controlState: item, + dependantControlName: key, + })); + + let updatedControlStates = {}; + if (dependantControls.length > 0) { + const updatedControls = dependantControls.map( + ({ controlState, dependantControlName }) => { + // overwrite state form data with current control value as the redux state will not + // have latest action value + const overWrittenState = { + ...state, + form_data: { + ...state.form_data, + [controlName]: action.value, + }, + }; + + return { + // Re run validation for dependant controls + controlState: getControlStateFromControlConfig( + controlState, + overWrittenState, + controlState?.value, + ), + dependantControlName, + }; + }, + ); + + updatedControlStates = updatedControls.reduce( + (acc, { controlState, dependantControlName }) => { + acc[dependantControlName] = { ...controlState }; + return acc; + }, + {}, + ); + } + return { ...state, form_data: new_form_data, @@ -227,6 +273,7 @@ export default function exploreReducer(state = {}, action) { }, }), ...rerenderedControls, + ...updatedControlStates, }, }; }, diff --git a/superset/common/query_context_factory.py b/superset/common/query_context_factory.py index 3e3fe2913946bb7455d445782bb44309f245be6d..7a9e9e150283d81a805ef730a1b96259a6ef51e5 100644 --- a/superset/common/query_context_factory.py +++ b/superset/common/query_context_factory.py @@ -65,12 +65,23 @@ class QueryContextFactory: # pylint: disable=too-few-public-methods result_type = result_type or ChartDataResultType.FULL result_format = result_format or ChartDataResultFormat.JSON + + # The server pagination var is extracted from form data as the + # row limit for server pagination is more + # This particular flag server_pagination only exists for table viz type + server_pagination = ( + bool(form_data.get("server_pagination")) if form_data else False + ) + queries_ = [ self._process_query_object( datasource_model_instance, form_data, self._query_object_factory.create( - result_type, datasource=datasource, **query_obj + result_type, + datasource=datasource, + server_pagination=server_pagination, + **query_obj, ), ) for query_obj in queries diff --git a/superset/common/query_object_factory.py b/superset/common/query_object_factory.py index 63d1ef09665cf6f0ab2e5c6207cf42014e777f7c..063ba023481e33bae1282a211f616afe5db45b3f 100644 --- a/superset/common/query_object_factory.py +++ b/superset/common/query_object_factory.py @@ -57,6 +57,7 @@ class QueryObjectFactory: # pylint: disable=too-few-public-methods row_limit: int | None = None, time_range: str | None = None, time_shift: str | None = None, + server_pagination: bool | None = None, **kwargs: Any, ) -> QueryObject: datasource_model_instance = None @@ -64,7 +65,12 @@ class QueryObjectFactory: # pylint: disable=too-few-public-methods datasource_model_instance = self._convert_to_model(datasource) processed_extras = self._process_extras(extras) result_type = kwargs.setdefault("result_type", parent_result_type) - row_limit = self._process_row_limit(row_limit, result_type) + + # Process row limit taking server pagination into account + row_limit = self._process_row_limit( + row_limit, result_type, server_pagination=server_pagination + ) + processed_time_range = self._process_time_range( time_range, kwargs.get("filters"), kwargs.get("columns") ) @@ -96,14 +102,27 @@ class QueryObjectFactory: # pylint: disable=too-few-public-methods return extras def _process_row_limit( - self, row_limit: int | None, result_type: ChartDataResultType + self, + row_limit: int | None, + result_type: ChartDataResultType, + server_pagination: bool | None = None, ) -> int: + """Process row limit taking into account server pagination. + + :param row_limit: The requested row limit + :param result_type: The type of result being processed + :param server_pagination: Whether server-side pagination is enabled + :return: The processed row limit + """ default_row_limit = ( self._config["SAMPLES_ROW_LIMIT"] if result_type == ChartDataResultType.SAMPLES else self._config["ROW_LIMIT"] ) - return apply_max_row_limit(row_limit or default_row_limit) + return apply_max_row_limit( + row_limit or default_row_limit, + server_pagination=server_pagination, + ) @staticmethod def _process_time_range( diff --git a/superset/config.py b/superset/config.py index a82c080b4299ef77c542b34674a874113ccb44a0..a01c380a7e989dcf0f5f7a420a61ff14f1687a4d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -969,6 +969,10 @@ MAPBOX_API_KEY = os.environ.get("MAPBOX_API_KEY", "") # Maximum number of rows returned for any analytical database query SQL_MAX_ROW = 100000 +# Maximum number of rows for any query with Server Pagination in Table Viz type +TABLE_VIZ_MAX_ROW_SERVER = 500000 + + # Maximum number of rows displayed in SQL Lab UI # Is set to avoid out of memory/localstorage issues in browsers. Does not affect # exported CSVs diff --git a/superset/utils/core.py b/superset/utils/core.py index 2b80c89f61268adcf1db105148bee257edc1fa29..f1792ac47a32d785334275cc3c207abbb3ecf440 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -1741,24 +1741,30 @@ def parse_boolean_string(bool_str: str | None) -> bool: def apply_max_row_limit( limit: int, - max_limit: int | None = None, + server_pagination: bool | None = None, ) -> int: """ - Override row limit if max global limit is defined + Override row limit based on server pagination setting :param limit: requested row limit - :param max_limit: Maximum allowed row limit + :param server_pagination: whether server-side pagination + is enabled, defaults to None :return: Capped row limit - >>> apply_max_row_limit(100000, 10) - 10 - >>> apply_max_row_limit(10, 100000) - 10 - >>> apply_max_row_limit(0, 10000) - 10000 - """ - if max_limit is None: - max_limit = current_app.config["SQL_MAX_ROW"] + >>> apply_max_row_limit(600000, server_pagination=True) # Server pagination + 500000 + >>> apply_max_row_limit(600000, server_pagination=False) # No pagination + 50000 + >>> apply_max_row_limit(5000) # No server_pagination specified + 5000 + >>> apply_max_row_limit(0) # Zero returns default max limit + 50000 + """ + max_limit = ( + current_app.config["TABLE_VIZ_MAX_ROW_SERVER"] + if server_pagination + else current_app.config["SQL_MAX_ROW"] + ) if limit != 0: return min(max_limit, limit) return max_limit diff --git a/superset/views/base.py b/superset/views/base.py index a598994f19f5de4dd05ac02785c4291d6ebab84e..43b4f776718142d851ac497d44aa6fbbe5574e5e 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -109,6 +109,7 @@ FRONTEND_CONF_KEYS = ( "JWT_ACCESS_CSRF_COOKIE_NAME", "SQLLAB_QUERY_RESULT_TIMEOUT", "SYNC_DB_PERMISSIONS_IN_ASYNC_MODE", + "TABLE_VIZ_MAX_ROW_SERVER", ) logger = logging.getLogger(__name__) diff --git a/tests/unit_tests/common/test_query_object_factory.py b/tests/unit_tests/common/test_query_object_factory.py index 8ff362dec362caa8d6c4896aa200b2b90b4354bf..379137a34c4f6bd622b8a87f83804763cbde9eb3 100644 --- a/tests/unit_tests/common/test_query_object_factory.py +++ b/tests/unit_tests/common/test_query_object_factory.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import Any, Optional +from typing import Any from unittest.mock import Mock from pytest import fixture # noqa: PT013 @@ -45,9 +45,15 @@ def connector_registry() -> Mock: return mock -def apply_max_row_limit(limit: int, max_limit: Optional[int] = None) -> int: - if max_limit is None: - max_limit = create_app_config()["SQL_MAX_ROW"] +def apply_max_row_limit( + limit: int, + server_pagination: bool | None = None, +) -> int: + max_limit = ( + create_app_config()["TABLE_VIZ_MAX_ROW_SERVER"] + if server_pagination + else create_app_config()["SQL_MAX_ROW"] + ) if limit != 0: return min(max_limit, limit) return max_limit