fix: row limits & row count labels are confusing (#27700)

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'

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,

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

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}
             />
           )}

superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx

@@ -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();
-});

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',

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>

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}

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}

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}

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();
 

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,
         },
       ],
     },

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,
       },
     },
   );

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>[][];
 }
 

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);');
 });
 

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 { rowcount = 0, limit = null, loading } = props;
-  const limitReached = rowcount === limit;
+  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>
   ) : (

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
 

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,

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
                 )

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:

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"),
             },
         )
 

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")