This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin 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 12fe2929a4 fix: row limits & row count labels are confusing (#27700)
12fe2929a4 is described below
commit 12fe2929a4a4b5627d9cff701a1e73644e78ac47
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Tue Apr 2 13:58:35 2024 -0700
fix: row limits & row count labels are confusing (#27700)
---
.../src/query/types/QueryResponse.ts | 1 +
.../plugins/plugin-chart-table/test/testData.ts | 1 +
.../Chart/DrillBy/useResultsTableView.tsx | 2 ++
.../src/explore/components/ChartPills.tsx | 2 +-
.../components/DataTableControl/RowCount.test.tsx | 36 ----------------------
.../explore/components/DataTableControl/index.tsx | 9 ------
.../components/DataTableControls.tsx | 5 +--
.../DataTablesPane/components/SamplesPane.tsx | 4 +++
.../components/SingleQueryResultPane.tsx | 2 ++
.../DataTablesPane/components/useResultsPane.tsx | 3 +-
.../DataTablesPane/test/DataTablesPane.test.tsx | 5 +++
.../test/ResultsPaneOnDashboard.test.tsx | 4 +++
.../DataTablesPane/test/SamplesPane.test.tsx | 2 ++
.../src/explore/components/DataTablesPane/types.ts | 2 ++
.../RowCountLabel/RowCountLabel.test.tsx | 2 +-
.../src/explore/components/RowCountLabel/index.tsx | 10 ++++--
superset/common/query_actions.py | 2 ++
superset/common/query_context_processor.py | 1 +
superset/common/utils/query_cache_manager.py | 5 +++
superset/models/helpers.py | 1 +
superset/views/core.py | 2 ++
superset/viz.py | 2 ++
22 files changed, 50 insertions(+), 53 deletions(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
b/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
index 1705814df1..93c7475d42 100644
---
a/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
+++
b/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
@@ -67,6 +67,7 @@ export interface ChartDataResponseResult {
is_cached: boolean;
query: string;
rowcount: number;
+ sql_rowcount: number;
stacktrace: string | null;
status:
| 'stopped'
diff --git a/superset-frontend/plugins/plugin-chart-table/test/testData.ts
b/superset-frontend/plugins/plugin-chart-table/test/testData.ts
index 24abc3381e..6c76e865e8 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/testData.ts
+++ b/superset-frontend/plugins/plugin-chart-table/test/testData.ts
@@ -80,6 +80,7 @@ const basicQueryResult: ChartDataResponseResult = {
is_cached: false,
query: 'SELECT ...',
rowcount: 100,
+ sql_rowcount: 100,
stacktrace: null,
status: 'success',
from_dttm: null,
diff --git
a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx
b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx
index b424b95ea5..bab24fdd47 100644
--- a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx
@@ -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
diff --git a/superset-frontend/src/explore/components/ChartPills.tsx
b/superset-frontend/src/explore/components/ChartPills.tsx
index 99f7c5aad5..489ae146aa 100644
--- a/superset-frontend/src/explore/components/ChartPills.tsx
+++ b/superset-frontend/src/explore/components/ChartPills.tsx
@@ -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}
/>
)}
diff --git
a/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
b/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
deleted file mode 100644
index 6ebbcef77b..0000000000
---
a/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
+++ /dev/null
@@ -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();
-});
diff --git
a/superset-frontend/src/explore/components/DataTableControl/index.tsx
b/superset-frontend/src/explore/components/DataTableControl/index.tsx
index 8dc4b1d014..60ca470e4a 100644
--- a/superset-frontend/src/explore/components/DataTableControl/index.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -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',
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx
b/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx
index 7a02114a2f..b1ff73e29e 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx
@@ -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>
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx
b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx
index b542aad996..eaae35bd2f 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx
@@ -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}
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx
b/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx
index c2614dfda6..b543167840 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx
@@ -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}
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
index 9b6b597c79..bae6b6fd7b 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
@@ -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}
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx
b/superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx
index 74358af959..40370638e5 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/test/DataTablesPane.test.tsx
@@ -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();
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
b/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
index 839126bb62..a6f9830d2e 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
@@ -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,
},
],
},
diff --git
a/superset-frontend/src/explore/components/DataTablesPane/test/SamplesPane.test.tsx
b/superset-frontend/src/explore/components/DataTablesPane/test/SamplesPane.test.tsx
index eed6e84808..02c421aeec 100644
---
a/superset-frontend/src/explore/components/DataTablesPane/test/SamplesPane.test.tsx
+++
b/superset-frontend/src/explore/components/DataTablesPane/test/SamplesPane.test.tsx
@@ -50,6 +50,8 @@ describe('SamplesPane', () => {
],
colnames: ['__timestamp', 'genre'],
coltypes: [2, 1],
+ rowcount: 2,
+ sql_rowcount: 2,
},
},
);
diff --git a/superset-frontend/src/explore/components/DataTablesPane/types.ts
b/superset-frontend/src/explore/components/DataTablesPane/types.ts
index 4e6062ba4a..4939b7d748 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/types.ts
+++ b/superset-frontend/src/explore/components/DataTablesPane/types.ts
@@ -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>[][];
}
diff --git
a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
index 2e7a44c15e..8e3e3b0c1d 100644
---
a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
+++
b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
@@ -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);');
});
diff --git a/superset-frontend/src/explore/components/RowCountLabel/index.tsx
b/superset-frontend/src/explore/components/RowCountLabel/index.tsx
index be41be0ba6..80fc362959 100644
--- a/superset-frontend/src/explore/components/RowCountLabel/index.tsx
+++ b/superset-frontend/src/explore/components/RowCountLabel/index.tsx
@@ -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>
) : (
diff --git a/superset/common/query_actions.py b/superset/common/query_actions.py
index d73a99d027..bdbccc78db 100644
--- a/superset/common/query_actions.py
+++ b/superset/common/query_actions.py
@@ -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
diff --git a/superset/common/query_context_processor.py
b/superset/common/query_context_processor.py
index d8b5bea4bb..f003f8ed30 100644
--- a/superset/common/query_context_processor.py
+++ b/superset/common/query_context_processor.py
@@ -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,
diff --git a/superset/common/utils/query_cache_manager.py
b/superset/common/utils/query_cache_manager.py
index a0fb65b20d..d2e6e07437 100644
--- a/superset/common/utils/query_cache_manager.py
+++ b/superset/common/utils/query_cache_manager.py
@@ -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
)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 450def33b2..4b22873903 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -584,6 +584,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:
diff --git a/superset/views/core.py b/superset/views/core.py
index 4faede0f34..287f71e548 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -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"),
},
)
diff --git a/superset/viz.py b/superset/viz.py
index 7c78f84228..447b36d53d 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -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")
