From 7ea53c5feef40a5ef033098747444a7eb89b5007 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 13 Nov 2023 14:19:46 -0800 Subject: [PATCH 1/2] web,graphql: Remove row limit --- graphql-server/schema.gql | 1 - .../src/sqlSelects/sqlSelect.enums.ts | 1 - .../src/sqlSelects/sqlSelect.model.ts | 16 +++------------- .../SqlDataTable/SqlMessage/SuccessMsg.tsx | 8 -------- .../SqlDataTable/SqlMessage/index.test.tsx | 18 ------------------ .../SqlDataTable/SqlMessage/index.tsx | 4 +--- web/gen/graphql-types.tsx | 1 - 7 files changed, 4 insertions(+), 45 deletions(-) diff --git a/graphql-server/schema.gql b/graphql-server/schema.gql index 7f024c8b..2ed8c8c8 100644 --- a/graphql-server/schema.gql +++ b/graphql-server/schema.gql @@ -235,7 +235,6 @@ enum QueryExecutionStatus { Success Error Timeout - RowLimit } type Status { diff --git a/graphql-server/src/sqlSelects/sqlSelect.enums.ts b/graphql-server/src/sqlSelects/sqlSelect.enums.ts index 2cee9249..095cb10c 100644 --- a/graphql-server/src/sqlSelects/sqlSelect.enums.ts +++ b/graphql-server/src/sqlSelects/sqlSelect.enums.ts @@ -4,7 +4,6 @@ export enum QueryExecutionStatus { Success, Error, Timeout, - RowLimit, } registerEnumType(QueryExecutionStatus, { name: "QueryExecutionStatus" }); diff --git a/graphql-server/src/sqlSelects/sqlSelect.model.ts b/graphql-server/src/sqlSelects/sqlSelect.model.ts index b7f87853..8d5feff9 100644 --- a/graphql-server/src/sqlSelects/sqlSelect.model.ts +++ b/graphql-server/src/sqlSelects/sqlSelect.model.ts @@ -60,23 +60,13 @@ export function fromSqlSelectRow( }; } - // Get first 201 rows to determine if limit has been reached - const sliced = doltRows.slice(0, 201); - const rows: row.Row[] = sliced.map(row.fromDoltRowRes); - if (!rows.length) { + if (!doltRows.length) { return res; } - const columns: column.Column[] = Object.keys(sliced[0]).map(c => { + const rows: row.Row[] = doltRows.map(row.fromDoltRowRes); + const columns: column.Column[] = Object.keys(doltRows[0]).map(c => { return { name: c, isPrimaryKey: false, type: "unknown" }; }); - if (rows.length > 200) { - return { - ...res, - columns, - rows: rows.slice(0, 200), - queryExecutionStatus: QueryExecutionStatus.RowLimit, - }; - } return { ...res, columns, rows }; } diff --git a/web/components/SqlDataTable/SqlMessage/SuccessMsg.tsx b/web/components/SqlDataTable/SqlMessage/SuccessMsg.tsx index 7cff1d2c..3b5662be 100644 --- a/web/components/SqlDataTable/SqlMessage/SuccessMsg.tsx +++ b/web/components/SqlDataTable/SqlMessage/SuccessMsg.tsx @@ -1,4 +1,3 @@ -import DoltLink from "@components/links/DoltLink"; import Link from "@components/links/Link"; import useIsDolt from "@hooks/useIsDolt"; import { SqlQueryParams } from "@lib/params"; @@ -41,7 +40,6 @@ export default function SuccessMsg(props: Props) { type ExecutionProps = { rowsLen: number; params: SqlQueryParams; - hitRowLimit?: boolean; }; export function ExecutionMessage(props: ExecutionProps) { @@ -55,12 +53,6 @@ export function ExecutionMessage(props: ExecutionProps) { on {props.params.refName} )} - {props.hitRowLimit && ( - - {" "} - (for unlimited query results download ) - - )}

); } diff --git a/web/components/SqlDataTable/SqlMessage/index.test.tsx b/web/components/SqlDataTable/SqlMessage/index.test.tsx index 32c8a0a6..b899184b 100644 --- a/web/components/SqlDataTable/SqlMessage/index.test.tsx +++ b/web/components/SqlDataTable/SqlMessage/index.test.tsx @@ -92,22 +92,4 @@ describe("test SqlMessage", () => { expect(await screen.findByText("master")).toBeVisible(); expect(screen.getByText("1 row selected")).toBeVisible(); }); - - it("renders row limit message", async () => { - render( - - - , - ); - - expect(await screen.findByText("master")).toBeVisible(); - expect(screen.getByText(/200 rows selected/)).toBeVisible(); - expect( - screen.getByText(/\(for unlimited query results download /), - ).toBeVisible(); - }); }); diff --git a/web/components/SqlDataTable/SqlMessage/index.tsx b/web/components/SqlDataTable/SqlMessage/index.tsx index ae406bb1..ea1d171d 100644 --- a/web/components/SqlDataTable/SqlMessage/index.tsx +++ b/web/components/SqlDataTable/SqlMessage/index.tsx @@ -4,7 +4,7 @@ import { QueryExecutionStatus } from "@gen/graphql-types"; import { isTimeoutError } from "@lib/errors/helpers"; import { SqlQueryParams } from "@lib/params"; import { isMultipleQueries } from "@lib/parseSqlQuery"; -import SuccessMsg, { ExecutionMessage } from "./SuccessMsg"; +import SuccessMsg from "./SuccessMsg"; import TimeoutMessage from "./TimeoutMsg"; import css from "./index.module.css"; import { improveGqlError } from "./utils"; @@ -43,8 +43,6 @@ export default function SqlMessage(props: Props) { return ; case QueryExecutionStatus.Timeout: return ; - case QueryExecutionStatus.RowLimit: - return ; case QueryExecutionStatus.Error: default: if (props.executionMessage && isTimeoutError(props.executionMessage)) { diff --git a/web/gen/graphql-types.tsx b/web/gen/graphql-types.tsx index 150ec68e..4cf8b936 100644 --- a/web/gen/graphql-types.tsx +++ b/web/gen/graphql-types.tsx @@ -509,7 +509,6 @@ export type QueryViewsArgs = { export enum QueryExecutionStatus { Error = 'Error', - RowLimit = 'RowLimit', Success = 'Success', Timeout = 'Timeout' } From d4a071b8c636932f7d04b4976eeae84484b795e4 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 13 Nov 2023 15:15:35 -0800 Subject: [PATCH 2/2] web,graphql: Remove default query --- graphql-server/src/branches/branch.queries.ts | 2 +- .../DatabaseTableHeaderMobile/index.tsx | 72 +++---------------- web/components/DatabaseTableHeader/index.tsx | 69 +++--------------- .../DatabaseTableHeader/utils.test.ts | 45 ++---------- web/components/DatabaseTableHeader/utils.ts | 60 ++++------------ web/components/SqlEditor/index.tsx | 25 ------- 6 files changed, 39 insertions(+), 234 deletions(-) diff --git a/graphql-server/src/branches/branch.queries.ts b/graphql-server/src/branches/branch.queries.ts index d168c98b..14dc9701 100644 --- a/graphql-server/src/branches/branch.queries.ts +++ b/graphql-server/src/branches/branch.queries.ts @@ -3,7 +3,7 @@ import { SortBranchesBy } from "./branch.enum"; export const branchQuery = `SELECT * FROM dolt_branches WHERE name=?`; export const getBranchesQuery = (sortBy?: SortBranchesBy) => - `SELECT * FROM dolt_branches ${getOrderByForBranches(sortBy)}LIMIT 200`; + `SELECT * FROM dolt_branches ${getOrderByForBranches(sortBy)}`; export const callNewBranch = `CALL DOLT_BRANCH(?, ?)`; diff --git a/web/components/DatabaseTableHeader/DatabaseTableHeaderMobile/index.tsx b/web/components/DatabaseTableHeader/DatabaseTableHeaderMobile/index.tsx index 1a36a2b8..cd8b2940 100644 --- a/web/components/DatabaseTableHeader/DatabaseTableHeaderMobile/index.tsx +++ b/web/components/DatabaseTableHeader/DatabaseTableHeaderMobile/index.tsx @@ -2,11 +2,7 @@ import Btn from "@components/Btn"; import Loader from "@components/Loader"; import MobileSqlViewer from "@components/SqlEditor/MobileSqlViewer"; import { useSqlEditorContext } from "@contexts/sqleditor"; -import { - ColumnForDataTableFragment, - useDataTableQuery, -} from "@gen/graphql-types"; -import { OptionalRefParams, RefParams } from "@lib/params"; +import { OptionalRefParams } from "@lib/params"; import { FaChevronDown } from "@react-icons/all-files/fa/FaChevronDown"; import { FaChevronUp } from "@react-icons/all-files/fa/FaChevronUp"; import { useEffect } from "react"; @@ -14,29 +10,15 @@ import Errors from "../Errors"; import { getEditorString, getSqlString } from "../utils"; import css from "./index.module.css"; -type Params = OptionalRefParams & { - q?: string; - tableName?: string; -}; -type RequireParams = RefParams & { - q?: string; - tableName: string; -}; - type Props = { - params: Params; + params: OptionalRefParams & { + q?: string; + tableName?: string; + }; empty?: boolean; }; -type InnerProps = Props & { - cols?: ColumnForDataTableFragment[]; -}; - -type QueryProps = Props & { - params: RequireParams; -}; - -function Inner(props: InnerProps) { +export default function DatabaseTableHeaderMobile(props: Props) { const sqlString = getSqlString( props.params.q, props.params.tableName, @@ -47,30 +29,20 @@ function Inner(props: InnerProps) { useSqlEditorContext(); useEffect(() => { - const sqlRes = getEditorString( + const sqlQuery = getEditorString( props.params.q, props.params.tableName, props.empty, - props.cols, ); - setEditorString(sqlRes.sqlQuery); - }, [ - props.cols, - props.empty, - props.params.q, - props.params.tableName, - setEditorString, - ]); + setEditorString(sqlQuery); + }, [props.empty, props.params.q, props.params.tableName]); - return props.empty ? ( -
Databases are read-only.
- ) : ( + return (
toggleSqlEditor()}> Query - {showSqlEditor ? ( ) : ( @@ -89,27 +61,3 @@ function Inner(props: InnerProps) {
); } - -function WithQuery(props: QueryProps) { - const res = useDataTableQuery({ - variables: props.params, - }); - if (res.loading) return ; - return ; -} - -export default function DatabaseTableHeaderMobile(props: Props) { - if (props.params.tableName && props.params.refName) { - return ( - - ); - } - return ; -} diff --git a/web/components/DatabaseTableHeader/index.tsx b/web/components/DatabaseTableHeader/index.tsx index 27e338eb..808acb16 100644 --- a/web/components/DatabaseTableHeader/index.tsx +++ b/web/components/DatabaseTableHeader/index.tsx @@ -2,14 +2,10 @@ import Btn from "@components/Btn"; import Loader from "@components/Loader"; import SqlEditor from "@components/SqlEditor"; import { useSqlEditorContext } from "@contexts/sqleditor"; -import { - ColumnForDataTableFragment, - useDataTableQuery, -} from "@gen/graphql-types"; -import { OptionalRefParams, RefParams } from "@lib/params"; +import { OptionalRefParams } from "@lib/params"; import { BiPencil } from "@react-icons/all-files/bi/BiPencil"; import dynamic from "next/dynamic"; -import { useEffect, useState } from "react"; +import { useEffect } from "react"; import Buttons from "./Buttons"; import Errors from "./Errors"; import css from "./index.module.css"; @@ -19,32 +15,15 @@ const AceEditor = dynamic(async () => import("@components/AceEditor"), { ssr: false, }); -type Params = OptionalRefParams & { - q?: string; - tableName?: string; -}; - -type RequireParams = RefParams & { - q?: string; - tableName: string; -}; - type Props = { - params: Params; + params: OptionalRefParams & { + q?: string; + tableName?: string; + }; empty?: boolean; }; -type InnerProps = Props & { - cols?: ColumnForDataTableFragment[]; -}; - -type QueryProps = Props & { - params: RequireParams; -}; - -function Inner(props: InnerProps) { - const [showDefaultQueryInfo, setShowDefaultQueryInfo] = useState(false); - +export default function DatabaseTableHeader(props: Props) { const sqlString = getSqlString( props.params.q, props.params.tableName, @@ -60,15 +39,13 @@ function Inner(props: InnerProps) { } = useSqlEditorContext(); useEffect(() => { - const sqlRes = getEditorString( + const sqlQuery = getEditorString( props.params.q, props.params.tableName, props.empty, - props.cols, ); - setEditorString(sqlRes.sqlQuery); - setShowDefaultQueryInfo(sqlRes.showDefaultQueryInfo); - }, [props.cols]); + setEditorString(sqlQuery); + }, [props.params.q, props.params.tableName, props.empty]); return (
@@ -86,7 +63,7 @@ function Inner(props: InnerProps) {
{showSqlEditor ? ( - + ) : ( ; - return ; -} - -export default function DatabaseTableHeader(props: Props) { - if (props.params.tableName && props.params.refName) { - return ( - - ); - } - return ; -} diff --git a/web/components/DatabaseTableHeader/utils.test.ts b/web/components/DatabaseTableHeader/utils.test.ts index 88e86664..81f2bbce 100644 --- a/web/components/DatabaseTableHeader/utils.test.ts +++ b/web/components/DatabaseTableHeader/utils.test.ts @@ -1,76 +1,41 @@ -import { ColumnForDataTableFragment } from "@gen/graphql-types"; import { sampleQuery } from "./utils"; -const idPKColumn: ColumnForDataTableFragment = { - name: "id", - isPrimaryKey: true, - type: "INT", -}; - -const pkPKColumn: ColumnForDataTableFragment = { - name: "pk2", - isPrimaryKey: true, - type: "INT", -}; - -const nameColumn: ColumnForDataTableFragment = { - name: "name", - isPrimaryKey: false, - type: "VARCHAR(16383)", -}; - describe("tests sampleQuery", () => { const tests: Array<{ desc: string; - cols: ColumnForDataTableFragment[]; expectedQuery: string; - expectedShowLink: boolean; tableName?: string; }> = [ { desc: "Keyless table", - cols: [nameColumn], - expectedQuery: `SELECT *\nFROM \`Keyless table\`\nLIMIT 200;\n`, - expectedShowLink: false, + expectedQuery: `SELECT * FROM \`Keyless table\``, tableName: "Keyless table", }, { desc: "One key and data", - cols: [idPKColumn, nameColumn], - expectedQuery: `SELECT *\nFROM \`One key and data table\`\nORDER BY \`id\` ASC\nLIMIT 200;\n`, - expectedShowLink: true, + expectedQuery: `SELECT * FROM \`One key and data table\``, tableName: "One key and data table", }, { desc: "Two Key", - cols: [idPKColumn, pkPKColumn], - expectedQuery: `SELECT *\nFROM \`Two Keys table\`\nORDER BY \`id\` ASC, \`pk2\` ASC\nLIMIT 200;\n`, - expectedShowLink: true, + expectedQuery: `SELECT * FROM \`Two Keys table\``, tableName: "Two Keys table", }, { desc: "Two Keys and data", - cols: [idPKColumn, pkPKColumn, nameColumn], - expectedQuery: `SELECT *\nFROM \`Two Keys and data table\`\nORDER BY \`id\` ASC, \`pk2\` ASC\nLIMIT 200;\n`, - expectedShowLink: true, + expectedQuery: `SELECT * FROM \`Two Keys and data table\``, tableName: "Two Keys and data table", }, { desc: "no table Name", - cols: [idPKColumn, pkPKColumn, nameColumn], - expectedShowLink: false, expectedQuery: `SHOW TABLES;\n`, }, ]; tests.forEach(test => { it(`tests ${test.desc} table default query`, () => { - const { sqlQuery, showDefaultQueryInfo } = sampleQuery( - test.tableName, - test.cols, - ); + const sqlQuery = sampleQuery(test.tableName); expect(sqlQuery).toContain(test.expectedQuery); - expect(showDefaultQueryInfo).toBe(test.expectedShowLink); }); }); }); diff --git a/web/components/DatabaseTableHeader/utils.ts b/web/components/DatabaseTableHeader/utils.ts index 58aac13b..b31fd409 100644 --- a/web/components/DatabaseTableHeader/utils.ts +++ b/web/components/DatabaseTableHeader/utils.ts @@ -1,14 +1,6 @@ -import { ColumnForDataTableFragment } from "@gen/graphql-types"; import { isDoltSystemTable } from "@lib/doltSystemTables"; -const sqlSelectRowLimit = 200; - -export const exampleCreateTable = `CREATE TABLE tablename (pk INT, col1 VARCHAR(255), PRIMARY KEY (pk));`; - -type EditorStringResult = { - sqlQuery: string; - showDefaultQueryInfo: boolean; -}; +const exampleCreateTable = `CREATE TABLE tablename (pk INT, col1 VARCHAR(255), PRIMARY KEY (pk));`; export function getSqlString( query?: string, @@ -28,53 +20,25 @@ export function getEditorString( query?: string, tableName?: string, empty?: boolean, - cols?: ColumnForDataTableFragment[], -): EditorStringResult { +): string { if (empty) { - return { - sqlQuery: sampleCreateQueryForEmpty(), - showDefaultQueryInfo: false, - }; + return sampleCreateQueryForEmpty(); } - return query - ? { sqlQuery: query, showDefaultQueryInfo: false } - : sampleQuery(tableName ?? "", cols); + return query ?? sampleQuery(tableName ?? ""); } -function getOrderByClause(cols?: ColumnForDataTableFragment[]) { - if (!cols || cols.length === 0) { - return ""; - } - const colCauses = cols - .filter(col => col.isPrimaryKey) - .map(c => `\`${c.name}\` ASC`) - .join(", "); - return colCauses !== "" ? `ORDER BY ${colCauses}` : ""; +function addEmptyLines(firstLine: string): string { + const lines = [firstLine]; + // eslint-disable-next-line no-empty + while (lines.push("") < 5) {} + return lines.join("\n"); } -export function sampleQuery( - tableName?: string, - cols?: ColumnForDataTableFragment[], -): EditorStringResult { - let lines: string[]; - const orderByClause = getOrderByClause(cols); +export function sampleQuery(tableName?: string): string { if (!tableName || isDoltSystemTable(tableName)) { - lines = ["SHOW TABLES;"]; - } else { - lines = [ - "SELECT *", - `FROM \`${tableName}\``, - orderByClause, - `LIMIT ${sqlSelectRowLimit};`, - ]; + return addEmptyLines("SHOW TABLES;"); } - lines = lines.filter(line => line !== ""); - // eslint-disable-next-line no-empty - while (lines.push("") < 5) {} - return { - sqlQuery: lines.join("\n"), - showDefaultQueryInfo: orderByClause !== "" && lines[0] !== "SHOW TABLES;", - }; + return addEmptyLines(`SELECT * FROM \`${tableName}\`;`); } export function sampleCreateQueryForEmpty(): string { diff --git a/web/components/SqlEditor/index.tsx b/web/components/SqlEditor/index.tsx index e735bc81..c9a6460e 100644 --- a/web/components/SqlEditor/index.tsx +++ b/web/components/SqlEditor/index.tsx @@ -1,7 +1,5 @@ -import Popup from "@components/Popup"; import { useSqlEditorContext } from "@contexts/sqleditor"; import { OptionalRefParams } from "@lib/params"; -import { BsFillQuestionCircleFill } from "@react-icons/all-files/bs/BsFillQuestionCircleFill"; import { debounce } from "lodash"; import dynamic from "next/dynamic"; import { useRef } from "react"; @@ -17,7 +15,6 @@ const AceEditor = dynamic(async () => import("@components/AceEditor"), { type Props = { params: OptionalRefParams; ["data-cy"]?: string; - showDefaultQueryInfo?: boolean; }; export default function SqlEditor(props: Props) { @@ -46,28 +43,6 @@ export default function SqlEditor(props: Props) {
- {props.showDefaultQueryInfo && ( -
- - {" "} -
Learn more about this query
- - } - > -

- This query represents the results you would get by running `SELECT - * FROM [tableName]`. SQL queries on DoltHub are limited to 200 - rows. The ORDER BY clause makes the results of the query - deterministic in the presence of the LIMIT. -

- -
- )} ); }