From fd15dff60eeb07c58efc9fa434809867cd41b336 Mon Sep 17 00:00:00 2001
From: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Date: Fri, 8 Jan 2021 21:07:51 +0100
Subject: [PATCH] perf: Optimize performance of Results and Samples tables on
 Explore (#12257)

---
 .../spec/javascripts/explore/utils_spec.jsx   | 15 ----------
 .../src/components/TableView/TableView.tsx    |  3 +-
 .../dataViewCommon/TableCollection.tsx        | 29 +++++++++----------
 .../explore/components/DataTableControl.tsx   | 12 ++++----
 .../src/explore/components/DataTablesPane.tsx |  9 ++----
 superset-frontend/src/explore/exploreUtils.js |  8 -----
 6 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/superset-frontend/spec/javascripts/explore/utils_spec.jsx b/superset-frontend/spec/javascripts/explore/utils_spec.jsx
index e41db222d..251475a21 100644
--- a/superset-frontend/spec/javascripts/explore/utils_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/utils_spec.jsx
@@ -23,7 +23,6 @@ import {
   buildV1ChartDataPayload,
   getExploreUrl,
   getExploreLongUrl,
-  getDataTablePageSize,
   shouldUseLegacyApi,
   getSimpleSQLExpression,
 } from 'src/explore/exploreUtils';
@@ -202,20 +201,6 @@ describe('exploreUtils', () => {
     });
   });
 
-  describe('getDataTablePageSize', () => {
-    it('divides samples data into pages dynamically', () => {
-      let pageSize;
-      pageSize = getDataTablePageSize(500);
-      expect(pageSize).toEqual(20);
-      pageSize = getDataTablePageSize(0);
-      expect(pageSize).toEqual(50);
-      pageSize = getDataTablePageSize(1);
-      expect(pageSize).toEqual(10000);
-      pageSize = getDataTablePageSize(1000000);
-      expect(pageSize).toEqual(5);
-    });
-  });
-
   describe('buildV1ChartDataPayload', () => {
     it('generate valid request payload despite no registered buildQuery', () => {
       const v1RequestPayload = buildV1ChartDataPayload({
diff --git a/superset-frontend/src/components/TableView/TableView.tsx b/superset-frontend/src/components/TableView/TableView.tsx
index 761a3414b..58b02bd33 100644
--- a/superset-frontend/src/components/TableView/TableView.tsx
+++ b/superset-frontend/src/components/TableView/TableView.tsx
@@ -56,6 +56,7 @@ const TableViewStyles = styled.div`
     display: flex;
     flex-direction: column;
     justify-content: center;
+    align-items: center;
   }
 
   .row-count-container {
@@ -165,4 +166,4 @@ const TableView = ({
   );
 };
 
-export default TableView;
+export default React.memo(TableView);
diff --git a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
index fe7f8a9b9..1c11f811c 100644
--- a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
+++ b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
@@ -52,9 +52,6 @@ export const Table = styled.table`
     background: ${({ theme }) => theme.colors.grayscale.light5};
     position: sticky;
     top: 0;
-
-    white-space: nowrap;
-
     &:first-of-type {
       padding-left: ${({ theme }) => theme.gridUnit * 4}px;
     }
@@ -205,17 +202,17 @@ export const Table = styled.table`
 
 Table.displayName = 'table';
 
-export default function TableCollection({
-  getTableProps,
-  getTableBodyProps,
-  prepareRow,
-  headerGroups,
-  columns,
-  rows,
-  loading,
-  highlightRowId,
-}: TableCollectionProps) {
-  return (
+export default React.memo(
+  ({
+    getTableProps,
+    getTableBodyProps,
+    prepareRow,
+    headerGroups,
+    columns,
+    rows,
+    loading,
+    highlightRowId,
+  }: TableCollectionProps) => (
     <Table
       {...getTableProps()}
       className="table table-hover"
@@ -314,5 +311,5 @@ export default function TableCollection({
           })}
       </tbody>
     </Table>
-  );
-}
+  ),
+);
diff --git a/superset-frontend/src/explore/components/DataTableControl.tsx b/superset-frontend/src/explore/components/DataTableControl.tsx
index cae88b0a2..30ffbd43c 100644
--- a/superset-frontend/src/explore/components/DataTableControl.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl.tsx
@@ -44,6 +44,12 @@ export const CopyButton = styled(Button)`
   }
 `;
 
+const CopyNode = (
+  <CopyButton buttonSize="xs">
+    <i className="fa fa-clipboard" />
+  </CopyButton>
+);
+
 export const CopyToClipboardButton = ({
   data,
 }: {
@@ -52,11 +58,7 @@ export const CopyToClipboardButton = ({
   <CopyToClipboard
     text={data ? prepareCopyToClipboardTabularData(data) : ''}
     wrapped={false}
-    copyNode={
-      <CopyButton buttonSize="xs">
-        <i className="fa fa-clipboard" />
-      </CopyButton>
-    }
+    copyNode={CopyNode}
   />
 );
 
diff --git a/superset-frontend/src/explore/components/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane.tsx
index a9403c37f..79ed09b46 100644
--- a/superset-frontend/src/explore/components/DataTablesPane.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane.tsx
@@ -24,7 +24,6 @@ import Loading from 'src/components/Loading';
 import TableView, { EmptyWrapperType } from 'src/components/TableView';
 import { getChartDataRequest } from 'src/chart/chartAction';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { getDataTablePageSize } from 'src/explore/exploreUtils';
 import {
   CopyToClipboardButton,
   FilterInput,
@@ -43,6 +42,8 @@ const NULLISH_RESULTS_STATE = {
   [RESULT_TYPES.samples]: undefined,
 };
 
+const DATA_TABLE_PAGE_SIZE = 50;
+
 const TableControlsWrapper = styled.div`
   display: flex;
   align-items: center;
@@ -189,9 +190,6 @@ export const DataTablesPane = ({
   };
 
   const renderDataTable = (type: string) => {
-    // restrict cell count to 10000 or min 5 rows to avoid crashing browser
-    const columnsLength = columns[type].length;
-    const pageSize = getDataTablePageSize(columnsLength);
     if (isLoading[type]) {
       return <Loading />;
     }
@@ -206,8 +204,7 @@ export const DataTablesPane = ({
         <TableView
           columns={columns[type]}
           data={filteredData[type]}
-          withPagination
-          pageSize={pageSize}
+          pageSize={DATA_TABLE_PAGE_SIZE}
           noDataText={t('No data')}
           emptyWrapperType={EmptyWrapperType.Small}
           className="table-condensed"
diff --git a/superset-frontend/src/explore/exploreUtils.js b/superset-frontend/src/explore/exploreUtils.js
index dd9d20b28..68c5a6fd2 100644
--- a/superset-frontend/src/explore/exploreUtils.js
+++ b/superset-frontend/src/explore/exploreUtils.js
@@ -251,14 +251,6 @@ export function postForm(url, payload, target = '_blank') {
   document.body.removeChild(hiddenForm);
 }
 
-export function getDataTablePageSize(columnsLength) {
-  let pageSize;
-  if (columnsLength) {
-    pageSize = Math.ceil(Math.max(5, 10000 / columnsLength));
-  }
-  return pageSize || 50;
-}
-
 export const exportChart = ({
   formData,
   resultFormat = 'json',