Merge #99450

99450: server,sql: remove usages of prettify on details pages r=maryliag a=maryliag The usage of `prettify_statement` and `Pretty` could cause OOM. Those were being used on statemen details and transaction insight details. This commit removes those usages and add a very basic formatting function that can be used on the SQLBox component. Part Of #91197 Release note (performance improvement): Removal of prettify usages that could cause OOM on Statement Details and Transaction Details page. Co-authored-by: maryliag <[email protected]>
cockroachdb · Mar 25, 2023 · 0fcc33b · 0fcc33b
2 parents 9a0d493 + 350f42b
commit 0fcc33b
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 19 deletions.
diff --git a/pkg/server/combined_statement_stats.go b/pkg/server/combined_statement_stats.go
@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/appstatspb"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -732,15 +731,7 @@ func getTotalStatementDetails(
 		return statement, serverError(ctx, err)
 	}
 
-	queryTree, err := parser.ParseOne(aggregatedMetadata.Query)
-	if err != nil {
-		return statement, serverError(ctx, err)
-	}
-	cfg := tree.DefaultPrettyCfg()
-	cfg.Align = tree.PrettyAlignOnly
-	cfg.LineWidth = tree.ConsoleLineWidth
-	aggregatedMetadata.FormattedQuery = cfg.Pretty(queryTree.AST)
-
+	aggregatedMetadata.FormattedQuery = aggregatedMetadata.Query
 	aggregatedMetadata.FingerprintID = string(tree.MustBeDString(row[3]))
 
 	statement = serverpb.StatementDetailsResponse_CollectedStatementSummary{

diff --git a/pkg/sql/appstatspb/app_stats.proto b/pkg/sql/appstatspb/app_stats.proto
@@ -226,9 +226,8 @@ message StatementStatisticsKey {
 
 message AggregatedStatementMetadata {
   optional string query = 1 [(gogoproto.nullable) = false];
-  // Formatted query is the return of the key_data.query after prettify_statement.
-  // The query above cannot be replaced by the formatted value, because is used as is for
-  // diagnostic bundle.
+  // Formatted query is the same value of query. It used to be formatted with prettify_statement,
+  // but until that function is improved (#91197), it should not be used.
   optional string formatted_query = 2 [(gogoproto.nullable) = false];
   optional string query_summary = 3 [(gogoproto.nullable) = false];
   optional string stmt_type = 4 [(gogoproto.nullable) = false];

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts
@@ -110,7 +110,7 @@ type FingerprintStmtsResponseColumns = {
 const fingerprintStmtsQuery = (stmtFingerprintIDs: string[]): string => `
 SELECT
   DISTINCT ON (fingerprint_id) encode(fingerprint_id, 'hex') AS statement_fingerprint_id,
-  prettify_statement(metadata ->> 'query', 108, 1, 1) AS query
+  (metadata ->> 'query') AS query
 FROM crdb_internal.statement_statistics
 WHERE encode(fingerprint_id, 'hex') =
       ANY ARRAY[ ${stmtFingerprintIDs.map(id => `'${id}'`).join(",")} ]`;

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx
@@ -159,7 +159,11 @@ export const StatementInsightDetails: React.FC<
           <section className={cx("section")}>
             <Row>
               <Col span={24}>
-                <SqlBox size={SqlBoxSize.custom} value={details?.query} />
+                <SqlBox
+                  size={SqlBoxSize.custom}
+                  value={details?.query}
+                  format={true}
+                />
               </Col>
             </Row>
           </section>

diff --git a/...s/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsOverviewTab.tsx b/...s/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsOverviewTab.tsx
@@ -131,7 +131,11 @@ the maximum number of statements was reached in the console.`;
         >
           <Row gutter={24}>
             <Col span={24}>
-              <SqlBox value={insightQueries} size={SqlBoxSize.custom} />
+              <SqlBox
+                value={insightQueries}
+                size={SqlBoxSize.custom}
+                format={true}
+              />
             </Col>
           </Row>
           {txnDetails && (

diff --git a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx
@@ -99,7 +99,11 @@ export class JobDetails extends React.Component<JobDetailsProps> {
       <>
         <Row gutter={24}>
           <Col className="gutter-row" span={24}>
-            <SqlBox value={job.description} size={SqlBoxSize.custom} />
+            <SqlBox
+              value={job.description}
+              size={SqlBoxSize.custom}
+              format={true}
+            />
           </Col>
         </Row>
         <Row gutter={24}>

diff --git a/pkg/ui/workspaces/cluster-ui/src/sql/box.tsx b/pkg/ui/workspaces/cluster-ui/src/sql/box.tsx
@@ -14,6 +14,7 @@ import classNames from "classnames/bind";
 
 import styles from "./sqlhighlight.module.scss";
 import { api as clusterUiApi } from "../index";
+import { FormatQuery } from "src/util";
 
 export enum SqlBoxSize {
   small = "small",
@@ -26,17 +27,21 @@ export interface SqlBoxProps {
   zone?: clusterUiApi.DatabaseDetailsResponse;
   className?: string;
   size?: SqlBoxSize;
+  format?: boolean;
 }
 
 const cx = classNames.bind(styles);
 
 export class SqlBox extends React.Component<SqlBoxProps> {
   preNode: React.RefObject<HTMLPreElement> = React.createRef();
   render(): React.ReactElement {
+    const value = this.props.format
+      ? FormatQuery(this.props.value)
+      : this.props.value;
     const sizeClass = this.props.size ? this.props.size : "";
     return (
       <div className={cx("box-highlight", this.props.className, sizeClass)}>
-        <Highlight {...this.props} />
+        <Highlight {...this.props} value={value} />
       </div>
     );
   }

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx
@@ -501,6 +501,7 @@ export class StatementDetails extends React.Component<
               <SqlBox
                 value={this.state.formattedQuery}
                 size={SqlBoxSize.custom}
+                format={true}
               />
             </Col>
           </Row>
@@ -684,6 +685,7 @@ export class StatementDetails extends React.Component<
               <SqlBox
                 value={this.state.formattedQuery}
                 size={SqlBoxSize.custom}
+                format={true}
               />
             </Col>
           </Row>
@@ -845,7 +847,11 @@ export class StatementDetails extends React.Component<
         <section className={cx("section")}>
           <Row gutter={24}>
             <Col className="gutter-row" span={24}>
-              <SqlBox value={formatted_query} size={SqlBoxSize.custom} />
+              <SqlBox
+                value={formatted_query}
+                size={SqlBoxSize.custom}
+                format={true}
+              />
             </Col>
           </Row>
           <p className={summaryCardStylesCx("summary--card__divider")} />

diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx
@@ -338,6 +338,7 @@ export class TransactionDetails extends React.Component<
                         <SqlBox
                           value={latestTransactionText}
                           className={transactionDetailsStylesCx("summary-card")}
+                          format={true}
                         />
                       </Col>
                     </Row>
@@ -458,6 +459,7 @@ export class TransactionDetails extends React.Component<
                       <SqlBox
                         value={latestTransactionText}
                         className={transactionDetailsStylesCx("summary-card")}
+                        format={true}
                       />
                     </Col>
                     <Col span={8}>

diff --git a/pkg/ui/workspaces/cluster-ui/src/util/format.ts b/pkg/ui/workspaces/cluster-ui/src/util/format.ts
@@ -352,3 +352,51 @@ export function EncodeDatabaseTableIndexUri(
 export function EncodeDatabaseUri(db: string): string {
   return `/database/${EncodeUriName(db)}`;
 }
+
+interface BreakLineReplacement {
+  [key: string]: string;
+}
+
+const breakLinesKeywords: BreakLineReplacement = {
+  " FROM ": " FROM ",
+  " WHERE ": "   WHERE ",
+  " AND ": "    AND ",
+  " ORDER ": " ORDER ",
+  " LIMIT ": " LIMIT ",
+  " JOIN ": "   JOIN ",
+  " ON ": "    ON ",
+  " VALUES ": "   VALUES ",
+};
+const LINE_BREAK_LIMIT = 100;
+
+export function FormatQuery(query: string): string {
+  if (query == null) {
+    return "";
+  }
+  Object.keys(breakLinesKeywords).forEach(key => {
+    query = query.replace(new RegExp(key, "g"), `\n${breakLinesKeywords[key]}`);
+  });
+  const lines = query.split("\n").map(line => {
+    if (line.length <= LINE_BREAK_LIMIT) {
+      return line;
+    }
+    return breakLongLine(line, LINE_BREAK_LIMIT);
+  });
+
+  return lines.join("\n");
+}
+
+function breakLongLine(line: string, limit: number): string {
+  if (line.length <= limit) {
+    return line;
+  }
+  const idxComma = line.indexOf(",", limit);
+  if (idxComma == -1) {
+    return line;
+  }
+
+  return `${line.substring(0, idxComma + 1)}\n${breakLongLine(
+    line.substring(idxComma + 1).trim(),
+    limit,
+  )}`;
+}