mirror of
https://github.com/apache/superset.git
synced 2024-09-17 11:09:47 -04:00
fix: row limits & row count labels are confusing (#27700)
(cherry picked from commit 12fe2929a4
)
This commit is contained in:
parent
026c75e018
commit
54942e2eaa
@ -67,6 +67,7 @@ export interface ChartDataResponseResult {
|
||||
is_cached: boolean;
|
||||
query: string;
|
||||
rowcount: number;
|
||||
sql_rowcount: number;
|
||||
stacktrace: string | null;
|
||||
status:
|
||||
| 'stopped'
|
||||
|
@ -80,6 +80,7 @@ const basicQueryResult: ChartDataResponseResult = {
|
||||
is_cached: false,
|
||||
query: 'SELECT ...',
|
||||
rowcount: 100,
|
||||
sql_rowcount: 100,
|
||||
stacktrace: null,
|
||||
status: 'success',
|
||||
from_dttm: null,
|
||||
|
@ -44,6 +44,7 @@ export const useResultsTableView = (
|
||||
<SingleQueryResultPane
|
||||
colnames={chartDataResult[0].colnames}
|
||||
coltypes={chartDataResult[0].coltypes}
|
||||
rowcount={chartDataResult[0].sql_rowcount}
|
||||
data={chartDataResult[0].data}
|
||||
dataSize={DATA_SIZE}
|
||||
datasourceId={datasourceId}
|
||||
@ -61,6 +62,7 @@ export const useResultsTableView = (
|
||||
colnames={res.colnames}
|
||||
coltypes={res.coltypes}
|
||||
data={res.data}
|
||||
rowcount={res.sql_rowcount}
|
||||
dataSize={DATA_SIZE}
|
||||
datasourceId={datasourceId}
|
||||
isVisible
|
||||
|
@ -66,7 +66,7 @@ export const ChartPills = forwardRef(
|
||||
>
|
||||
{!isLoading && firstQueryResponse && (
|
||||
<RowCountLabel
|
||||
rowcount={Number(firstQueryResponse.rowcount) || 0}
|
||||
rowcount={Number(firstQueryResponse.sql_rowcount) || 0}
|
||||
limit={Number(rowLimit) || 0}
|
||||
/>
|
||||
)}
|
||||
|
@ -1,36 +0,0 @@
|
||||
/**
|
||||
* 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 React from 'react';
|
||||
import { render, screen } from 'spec/helpers/testing-library';
|
||||
import { RowCount } from '.';
|
||||
|
||||
test('Render a RowCount with a single row', () => {
|
||||
render(<RowCount data={[{}]} loading={false} />);
|
||||
expect(screen.getByText('1 row')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('Render a RowCount with multiple rows', () => {
|
||||
render(<RowCount data={[{}, {}, {}]} loading={false} />);
|
||||
expect(screen.getByText('3 rows')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('Render a RowCount on loading', () => {
|
||||
render(<RowCount data={[{}, {}, {}]} loading />);
|
||||
expect(screen.getByText('Loading...')).toBeInTheDocument();
|
||||
});
|
@ -44,7 +44,6 @@ import Button from 'src/components/Button';
|
||||
import Popover from 'src/components/Popover';
|
||||
import { prepareCopyToClipboardTabularData } from 'src/utils/common';
|
||||
import CopyToClipboard from 'src/components/CopyToClipboard';
|
||||
import RowCountLabel from 'src/explore/components/RowCountLabel';
|
||||
import { getTimeColumns, setTimeColumns } from './utils';
|
||||
|
||||
export const CellNull = styled('span')`
|
||||
@ -118,14 +117,6 @@ export const FilterInput = ({
|
||||
);
|
||||
};
|
||||
|
||||
export const RowCount = ({
|
||||
data,
|
||||
loading,
|
||||
}: {
|
||||
data?: Record<string, any>[];
|
||||
loading: boolean;
|
||||
}) => <RowCountLabel rowcount={data?.length ?? 0} loading={loading} />;
|
||||
|
||||
enum FormatPickerValue {
|
||||
Formatted = 'formatted',
|
||||
Original = 'original',
|
||||
|
@ -22,10 +22,10 @@ import { css, GenericDataType, styled } from '@superset-ui/core';
|
||||
import {
|
||||
CopyToClipboardButton,
|
||||
FilterInput,
|
||||
RowCount,
|
||||
} from 'src/explore/components/DataTableControl';
|
||||
import { applyFormattingToTabularData } from 'src/utils/common';
|
||||
import { getTimeColumns } from 'src/explore/components/DataTableControl/utils';
|
||||
import RowCountLabel from 'src/explore/components/RowCountLabel';
|
||||
import { TableControlsProps } from '../types';
|
||||
|
||||
export const TableControlsWrapper = styled.div`
|
||||
@ -47,6 +47,7 @@ export const TableControls = ({
|
||||
onInputChange,
|
||||
columnNames,
|
||||
columnTypes,
|
||||
rowcount,
|
||||
isLoading,
|
||||
}: TableControlsProps) => {
|
||||
const originalTimeColumns = getTimeColumns(datasourceId);
|
||||
@ -74,7 +75,7 @@ export const TableControls = ({
|
||||
align-items: center;
|
||||
`}
|
||||
>
|
||||
<RowCount data={data} loading={isLoading} />
|
||||
<RowCountLabel rowcount={rowcount} loading={isLoading} />
|
||||
<CopyToClipboardButton data={formattedData} columns={columnNames} />
|
||||
</div>
|
||||
</TableControlsWrapper>
|
||||
|
@ -48,6 +48,7 @@ export const SamplesPane = ({
|
||||
const [colnames, setColnames] = useState<string[]>([]);
|
||||
const [coltypes, setColtypes] = useState<GenericDataType[]>([]);
|
||||
const [isLoading, setIsLoading] = useState<boolean>(false);
|
||||
const [rowcount, setRowCount] = useState<number>(0);
|
||||
const [responseError, setResponseError] = useState<string>('');
|
||||
const datasourceId = useMemo(
|
||||
() => `${datasource.id}__${datasource.type}`,
|
||||
@ -66,6 +67,7 @@ export const SamplesPane = ({
|
||||
setData(ensureIsArray(response.data));
|
||||
setColnames(ensureIsArray(response.colnames));
|
||||
setColtypes(ensureIsArray(response.coltypes));
|
||||
setRowCount(response.rowcount);
|
||||
setResponseError('');
|
||||
cache.add(datasource);
|
||||
if (queryForce && actions) {
|
||||
@ -108,6 +110,7 @@ export const SamplesPane = ({
|
||||
data={filteredData}
|
||||
columnNames={colnames}
|
||||
columnTypes={coltypes}
|
||||
rowcount={rowcount}
|
||||
datasourceId={datasourceId}
|
||||
onInputChange={input => setFilterText(input)}
|
||||
isLoading={isLoading}
|
||||
@ -128,6 +131,7 @@ export const SamplesPane = ({
|
||||
data={filteredData}
|
||||
columnNames={colnames}
|
||||
columnTypes={coltypes}
|
||||
rowcount={rowcount}
|
||||
datasourceId={datasourceId}
|
||||
onInputChange={input => setFilterText(input)}
|
||||
isLoading={isLoading}
|
||||
|
@ -30,6 +30,7 @@ export const SingleQueryResultPane = ({
|
||||
data,
|
||||
colnames,
|
||||
coltypes,
|
||||
rowcount,
|
||||
datasourceId,
|
||||
dataSize = 50,
|
||||
isVisible,
|
||||
@ -55,6 +56,7 @@ export const SingleQueryResultPane = ({
|
||||
data={filteredData}
|
||||
columnNames={colnames}
|
||||
columnTypes={coltypes}
|
||||
rowcount={rowcount}
|
||||
datasourceId={datasourceId}
|
||||
onInputChange={input => setFilterText(input)}
|
||||
isLoading={false}
|
||||
|
@ -119,6 +119,7 @@ export const useResultsPane = ({
|
||||
data={[]}
|
||||
columnNames={[]}
|
||||
columnTypes={[]}
|
||||
rowcount={0}
|
||||
datasourceId={queryFormData.datasource}
|
||||
onInputChange={() => {}}
|
||||
isLoading={false}
|
||||
@ -135,7 +136,6 @@ export const useResultsPane = ({
|
||||
<EmptyStateMedium image="document.svg" title={title} />,
|
||||
);
|
||||
}
|
||||
|
||||
return resultResp
|
||||
.slice(0, queryCount)
|
||||
.map((result, idx) => (
|
||||
@ -143,6 +143,7 @@ export const useResultsPane = ({
|
||||
data={result.data}
|
||||
colnames={result.colnames}
|
||||
coltypes={result.coltypes}
|
||||
rowcount={result.rowcount}
|
||||
dataSize={dataSize}
|
||||
datasourceId={queryFormData.datasource}
|
||||
key={idx}
|
||||
|
@ -93,6 +93,8 @@ describe('DataTablesPane', () => {
|
||||
data: [{ __timestamp: 1230768000000, genre: 'Action' }],
|
||||
colnames: ['__timestamp', 'genre'],
|
||||
coltypes: [2, 1],
|
||||
rowcount: 1,
|
||||
sql_rowcount: 1,
|
||||
},
|
||||
],
|
||||
},
|
||||
@ -125,6 +127,8 @@ describe('DataTablesPane', () => {
|
||||
],
|
||||
colnames: ['__timestamp', 'genre'],
|
||||
coltypes: [2, 1],
|
||||
rowcount: 2,
|
||||
sql_rowcount: 2,
|
||||
},
|
||||
],
|
||||
},
|
||||
@ -135,6 +139,7 @@ describe('DataTablesPane', () => {
|
||||
});
|
||||
userEvent.click(screen.getByText('Results'));
|
||||
expect(await screen.findByText('2 rows')).toBeVisible();
|
||||
|
||||
expect(screen.getByText('Action')).toBeVisible();
|
||||
expect(screen.getByText('Horror')).toBeVisible();
|
||||
|
||||
|
@ -50,6 +50,8 @@ describe('ResultsPaneOnDashboard', () => {
|
||||
],
|
||||
colnames: ['__timestamp', 'genre'],
|
||||
coltypes: [2, 1],
|
||||
rowcount: 2,
|
||||
sql_rowcount: 2,
|
||||
},
|
||||
],
|
||||
},
|
||||
@ -78,6 +80,8 @@ describe('ResultsPaneOnDashboard', () => {
|
||||
data: [{ genre: 'Action' }, { genre: 'Horror' }],
|
||||
colnames: ['genre'],
|
||||
coltypes: [1],
|
||||
rowcount: 2,
|
||||
sql_rowcount: 2,
|
||||
},
|
||||
],
|
||||
},
|
||||
|
@ -50,6 +50,8 @@ describe('SamplesPane', () => {
|
||||
],
|
||||
colnames: ['__timestamp', 'genre'],
|
||||
coltypes: [2, 1],
|
||||
rowcount: 2,
|
||||
sql_rowcount: 2,
|
||||
},
|
||||
},
|
||||
);
|
||||
|
@ -71,11 +71,13 @@ export interface TableControlsProps {
|
||||
columnNames: string[];
|
||||
columnTypes: GenericDataType[];
|
||||
isLoading: boolean;
|
||||
rowcount: number;
|
||||
}
|
||||
|
||||
export interface QueryResultInterface {
|
||||
colnames: string[];
|
||||
coltypes: GenericDataType[];
|
||||
rowcount: number;
|
||||
data: Record<string, any>[][];
|
||||
}
|
||||
|
||||
|
@ -52,7 +52,7 @@ test('RowCountLabel renders limit with danger and tooltip', async () => {
|
||||
expect(screen.getByText(expectedText)).toBeInTheDocument();
|
||||
userEvent.hover(screen.getByText(expectedText));
|
||||
const tooltip = await screen.findByRole('tooltip');
|
||||
expect(tooltip).toHaveTextContent('Limit reached');
|
||||
expect(tooltip).toHaveTextContent('The row limit');
|
||||
expect(tooltip).toHaveStyle('background: rgba(0, 0, 0, 0.902);');
|
||||
});
|
||||
|
||||
|
@ -28,9 +28,13 @@ type RowCountLabelProps = {
|
||||
loading?: boolean;
|
||||
};
|
||||
|
||||
const limitReachedMsg = t(
|
||||
'The row limit set for the chart was reached. The chart may show partial data.',
|
||||
);
|
||||
|
||||
export default function RowCountLabel(props: RowCountLabelProps) {
|
||||
const { rowcount = 0, limit, loading } = props;
|
||||
const limitReached = rowcount === limit;
|
||||
const { rowcount = 0, limit = null, loading } = props;
|
||||
const limitReached = limit && rowcount >= limit;
|
||||
const type =
|
||||
limitReached || (rowcount === 0 && !loading) ? 'danger' : 'default';
|
||||
const formattedRowCount = getNumberFormatter()(rowcount);
|
||||
@ -46,7 +50,7 @@ export default function RowCountLabel(props: RowCountLabelProps) {
|
||||
</Label>
|
||||
);
|
||||
return limitReached ? (
|
||||
<Tooltip id="tt-rowcount-tooltip" title={<span>{t('Limit reached')}</span>}>
|
||||
<Tooltip id="tt-rowcount-tooltip" title={<span>{limitReachedMsg}</span>}>
|
||||
{label}
|
||||
</Tooltip>
|
||||
) : (
|
||||
|
@ -135,6 +135,8 @@ def _get_full(
|
||||
"data": payload.get("data"),
|
||||
"colnames": payload.get("colnames"),
|
||||
"coltypes": payload.get("coltypes"),
|
||||
"rowcount": payload.get("rowcount"),
|
||||
"sql_rowcount": payload.get("sql_rowcount"),
|
||||
}
|
||||
return payload
|
||||
|
||||
|
@ -194,6 +194,7 @@ class QueryContextProcessor:
|
||||
"status": cache.status,
|
||||
"stacktrace": cache.stacktrace,
|
||||
"rowcount": len(cache.df.index),
|
||||
"sql_rowcount": cache.sql_rowcount,
|
||||
"from_dttm": query_obj.from_dttm,
|
||||
"to_dttm": query_obj.to_dttm,
|
||||
"label_map": label_map,
|
||||
|
@ -64,6 +64,7 @@ class QueryCacheManager:
|
||||
is_cached: bool | None = None,
|
||||
cache_dttm: str | None = None,
|
||||
cache_value: dict[str, Any] | None = None,
|
||||
sql_rowcount: int | None = None,
|
||||
) -> None:
|
||||
self.df = df
|
||||
self.query = query
|
||||
@ -79,6 +80,7 @@ class QueryCacheManager:
|
||||
self.is_cached = is_cached
|
||||
self.cache_dttm = cache_dttm
|
||||
self.cache_value = cache_value
|
||||
self.sql_rowcount = sql_rowcount
|
||||
|
||||
# pylint: disable=too-many-arguments
|
||||
def set_query_result(
|
||||
@ -102,6 +104,7 @@ class QueryCacheManager:
|
||||
self.rejected_filter_columns = query_result.rejected_filter_columns
|
||||
self.error_message = query_result.error_message
|
||||
self.df = query_result.df
|
||||
self.sql_rowcount = query_result.sql_rowcount
|
||||
self.annotation_data = {} if annotation_data is None else annotation_data
|
||||
|
||||
if self.status != QueryStatus.FAILED:
|
||||
@ -117,6 +120,7 @@ class QueryCacheManager:
|
||||
"applied_filter_columns": self.applied_filter_columns,
|
||||
"rejected_filter_columns": self.rejected_filter_columns,
|
||||
"annotation_data": self.annotation_data,
|
||||
"sql_rowcount": self.sql_rowcount,
|
||||
}
|
||||
if self.is_loaded and key and self.status != QueryStatus.FAILED:
|
||||
self.set(
|
||||
@ -167,6 +171,7 @@ class QueryCacheManager:
|
||||
query_cache.status = QueryStatus.SUCCESS
|
||||
query_cache.is_loaded = True
|
||||
query_cache.is_cached = cache_value is not None
|
||||
query_cache.sql_rowcount = cache_value.get("sql_rowcount", None)
|
||||
query_cache.cache_dttm = (
|
||||
cache_value["dttm"] if cache_value is not None else None
|
||||
)
|
||||
|
@ -580,6 +580,7 @@ class QueryResult: # pylint: disable=too-few-public-methods
|
||||
self.errors = errors or []
|
||||
self.from_dttm = from_dttm
|
||||
self.to_dttm = to_dttm
|
||||
self.sql_rowcount = len(self.df.index) if not self.df.empty else 0
|
||||
|
||||
|
||||
class ExtraJSONMixin:
|
||||
|
@ -167,6 +167,8 @@ class Superset(BaseSupersetView):
|
||||
"data": payload["df"].to_dict("records"),
|
||||
"colnames": payload.get("colnames"),
|
||||
"coltypes": payload.get("coltypes"),
|
||||
"rowcount": payload.get("rowcount"),
|
||||
"sql_rowcount": payload.get("sql_rowcount"),
|
||||
},
|
||||
)
|
||||
|
||||
|
@ -262,6 +262,8 @@ class BaseViz: # pylint: disable=too-many-public-methods
|
||||
"data": payload["df"].to_dict(orient="records"),
|
||||
"colnames": payload.get("colnames"),
|
||||
"coltypes": payload.get("coltypes"),
|
||||
"rowcount": payload.get("rowcount"),
|
||||
"sql_rowcount": payload.get("sql_rowcount"),
|
||||
}
|
||||
|
||||
@deprecated(deprecated_in="3.0")
|
||||
|
Loading…
Reference in New Issue
Block a user