Merge pull request #3358 from ingef/feature/rework-table-rendering

Simplify Table rendering. Move allocations out of Cell-functions, avoiding unnecessary allocations

frontend/src/js/api/api.ts

@@ -418,7 +418,7 @@ export const useGetResult = () => {
     [authToken],
   );
   return useCallback(
-    (queryId: string, limit = 1000) => {
+    (queryId: string, limit: number) => {
       const url =
         `/result/arrow/${queryId}.arrs?` +
         new URLSearchParams({ limit: limit.toString() });

frontend/src/js/preview/Table.tsx

@@ -6,8 +6,7 @@ import {
   Vector,
 } from "apache-arrow";
 import RcTable from "rc-table";
-import { DefaultRecordType } from "rc-table/lib/interface";
-import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react";
+import { memo, useMemo, useRef } from "react";
 import { GetQueryResponseDoneT, GetQueryResponseT } from "../api/types";
 import { useCustomTableRenderers } from "./tableUtils";
 
@@ -69,73 +68,32 @@ export default memo(function Table({
 
   const columns = useMemo(
     () =>
-      arrowReader.schema?.fields.map((field) => ({
-        title: field.name.charAt(0).toUpperCase() + field.name.slice(1),
-        dataIndex: field.name,
-        key: field.name,
-        render: (value: string | Vector) => {
-          return typeof value === "string" ? (
-            <span title={value as string}>{value}</span>
-          ) : (
-            value
-          );
-        },
-      })),
-    [arrowReader.schema],
-  );
-
-  const parseTableRows = useCallback(
-    (data: Vector[]) => {
-      const nextRows = [] as DefaultRecordType[];
-      data.forEach((dataEntry: Vector) => {
-        const parsedValues = Object.fromEntries(
-          Object.entries(dataEntry.toJSON()).map(([key, value]) => {
-            const parsedValue =
-              getRenderFunctionByFieldName(key)?.(value) ?? value;
-            return [key, parsedValue];
-          }),
-        );
-        nextRows.push(parsedValues);
-      });
-
-      return nextRows;
-    },
-    [getRenderFunctionByFieldName],
+      arrowReader.schema?.fields.map((field) => {
+        const renderer = getRenderFunctionByFieldName(field.name);
+
+        return {
+          title: field.name.charAt(0).toUpperCase() + field.name.slice(1),
+          dataIndex: field.name,
+          key: field.name,
+          render: (value: string | Vector) => {
+            const rendered = renderer(value);
+            return <span title={rendered}>{rendered}</span>;
+          },
+        };
+      }),
+    [arrowReader.schema, getRenderFunctionByFieldName],
   );
 
   const loadedTableData = useMemo(
-    () => parseTableRows(new ArrowTable(initialTableData.value).toArray()),
-    [initialTableData, parseTableRows],
+    () => new ArrowTable(initialTableData.value).toArray(),
+    [initialTableData],
   );
-  const [visibleTableRows, setVisibleTableRows] = useState(50);
-
-  useEffect(() => {
-    const eventFunction = async () => {
-      const div = rootRef.current;
-      if (!div) {
-        return;
-      }
-      const maxScroll =
-        (div.parentElement?.scrollHeight || div.scrollHeight) -
-        window.innerHeight;
-      const thresholdTriggered =
-        (div.parentElement?.scrollTop || div.scrollTop) / maxScroll > 0.9;
-      if (thresholdTriggered) {
-        setVisibleTableRows((rowCount) =>
-          Math.min(rowCount + 50, loadedTableData.length),
-        );
-      }
-    };
-
-    window.addEventListener("scroll", eventFunction, true);
-    return () => window.removeEventListener("scroll", eventFunction, true);
-  }, [loadedTableData, visibleTableRows, arrowReader]);
 
   return (
     <Root ref={rootRef}>
       <RcTable
         columns={columns}
-        data={loadedTableData.slice(0, visibleTableRows)}
+        data={loadedTableData}
         rowKey={(_, index) => `previewtable_row_${index}`}
         components={{
           table: StyledTable,

frontend/src/js/preview/actions.ts

@@ -81,7 +81,7 @@ export function useLoadPreviewData() {
 
     try {
       const arrowReader = await AsyncRecordBatchStreamReader.from(
-        getResult(queryId),
+        getResult(queryId, 100),
       );
       const loadInitialData = async () => {
         await arrowReader.open();

frontend/src/js/preview/tableUtils.ts

@@ -4,12 +4,7 @@ import { useTranslation } from "react-i18next";
 import { useSelector } from "react-redux";
 import { CurrencyConfigT, GetQueryResponseDoneT } from "../api/types";
 import { StateT } from "../app/reducers";
-import {
-  NUMBER_TYPES,
-  formatDate,
-  formatNumber,
-  toFullLocaleDateString,
-} from "./util";
+import { NUMBER_TYPES, currencyFromSymbol } from "./util";
 
 export type CellValue = string | Vector;
 
@@ -20,7 +15,18 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) {
   );
 
   const getRenderFunction = useCallback(
-    (cellType: string): ((value: CellValue) => string) | undefined => {
+    (cellType: string): ((value: CellValue) => string) => {
+      const dateFormatter = new Intl.DateTimeFormat(navigator.language, {
+        day: "2-digit",
+        month: "2-digit",
+        year: "numeric",
+      });
+
+      const currencyFormatter = new Intl.NumberFormat(navigator.language, {
+        style: "currency",
+        currency: currencyFromSymbol(currencyConfig.unit),
+      });
+
       if (cellType.indexOf("LIST") == 0) {
         const listType = cellType.match(/LIST\[(?<listtype>.*)\]/)?.groups?.[
           "listtype"
@@ -30,59 +36,65 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) {
           return (value) =>
             value
               ? (value as Vector)
-                  .toArray()
-                  .map((listItem: string) =>
-                    listTypeRenderFunction
-                      ? listTypeRenderFunction(listItem)
-                      : listItem,
-                  )
+                  .toArray() // This is somewhat slow, but for-loop produces bogus values
+                  .map(listTypeRenderFunction)
                   .join(", ")
               : null;
         }
       } else if (NUMBER_TYPES.includes(cellType)) {
+        const numnberFormatter = new Intl.NumberFormat(navigator.language, {
+          maximumFractionDigits: 2,
+          minimumFractionDigits: cellType == "INTEGER" ? 0 : 2,
+        });
+
         return (value) => {
-          const num = parseFloat(value as string);
-          return isNaN(num) ? "" : formatNumber(num);
+          if (value && !isNaN(value as unknown as number)) {
+            return numnberFormatter.format(value as unknown as number);
+          }
+          return "";
         };
       } else if (cellType == "DATE") {
-        return (value) =>
-          value instanceof Date
-            ? toFullLocaleDateString(value)
-            : formatDate(value as string);
+        return (value) => dateFormatter.format(value as unknown as Date);
       } else if (cellType == "DATE_RANGE") {
         return (value) => {
-          const dateRange = (value as Vector).toJSON() as unknown as {
-            min: Date;
-            max: Date;
-          };
-          const min = toFullLocaleDateString(dateRange.min);
-          const max = toFullLocaleDateString(dateRange.max);
-          return min == max ? min : `${min} - ${max}`;
+          const vector = value as unknown as { min: Date; max: Date };
+
+          const min = dateFormatter.format(vector.min);
+          const max = dateFormatter.format(vector.max);
+
+          if (min == max) {
+            return min;
+          }
+
+          return `${min} - ${max}`;
         };
       } else if (cellType == "MONEY") {
         return (value) => {
-          const num = parseFloat(value as string) / 100;
-          return isNaN(num)
-            ? ""
-            : `${formatNumber(num, { forceFractionDigits: true })} ${
-                currencyConfig.unit
-              }`;
+          if (value && !isNaN(value as unknown as number)) {
+            return currencyFormatter.format(value as unknown as number);
+          }
+          return "";
         };
       } else if (cellType == "BOOLEAN") {
         return (value) => (value ? t("common.true") : t("common.false"));
       }
+
+      return (value) => (value ? (value as string) : "");
     },
     [currencyConfig.unit, t],
   );
 
   const getRenderFunctionByFieldName = useCallback(
-    (fieldName: string): ((value: CellValue) => string) | undefined => {
+    (fieldName: string): ((value: CellValue) => string) => {
       const cellType = (
         queryData as GetQueryResponseDoneT
       ).columnDescriptions?.find((x) => x.label == fieldName)?.type;
+
       if (cellType) {
         return getRenderFunction(cellType);
       }
+
+      return (value) => (value ? (value as string) : "");
     },
     [getRenderFunction, queryData],
   );

frontend/src/js/preview/util.ts

@@ -1,11 +1,16 @@
-import { t } from "i18next";
 import { BarStatistics, DateStatistics, PreviewStatistics } from "../api/types";
-import { parseDate } from "../common/helpers/dateHelper";
 
 export const NUMBER_TYPES = ["NUMERIC", "INTEGER"];
 
 export const NUMBER_STATISTICS_TYPES = [...NUMBER_TYPES, "MONEY"];
 
+export function currencyFromSymbol(symbol: string): string {
+  // TODO: this is a workaround until the backend sends currency-codes
+  if (symbol === "€") return "EUR";
+
+  return "USD";
+}
+
 export function formatNumber(
   num: number,
   {
@@ -19,22 +24,6 @@ export function formatNumber(
   }).format(num);
 }
 
-export function formatDate(date: string | undefined) {
-  if (date) {
-    const parsedDate = parseDate(date, "yyyy-MM-dd");
-    return parsedDate ? toFullLocaleDateString(parsedDate) : date;
-  }
-  return t("preview.dateError");
-}
-
-export function toFullLocaleDateString(date: Date) {
-  return date.toLocaleDateString(navigator.language, {
-    day: "2-digit",
-    month: "2-digit",
-    year: "numeric",
-  });
-}
-
 export function previewStatsIsBarStats(
   stats: PreviewStatistics,
 ): stats is BarStatistics {