diff --git a/apps/studio/components/grid/components/grid/Grid.tsx b/apps/studio/components/grid/components/grid/Grid.tsx index 73215f2e232..b24fa4a2c86 100644 --- a/apps/studio/components/grid/components/grid/Grid.tsx +++ b/apps/studio/components/grid/components/grid/Grid.tsx @@ -1,10 +1,6 @@ import type { PostgresColumn } from '@supabase/postgres-meta' -import { forwardRef, memo, Ref, useMemo, useRef } from 'react' -import DataGrid, { CalculatedColumn, DataGridHandle } from 'react-data-grid' -import { ref as valtioRef } from 'valtio' - -import { useTableFilter } from 'components/grid/hooks/useTableFilter' import { handleCopyCell } from 'components/grid/SupabaseGrid.utils' +import { useTableFilter } from 'components/grid/hooks/useTableFilter' import { formatForeignKeys } from 'components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils' import { useForeignKeyConstraintsQuery } from 'data/database/foreign-key-constraints-query' import { ENTITY_TYPE } from 'data/entity-types/entity-type-constants' @@ -13,15 +9,20 @@ import { useSendEventMutation } from 'data/telemetry/send-event-mutation' import { useSelectedOrganizationQuery } from 'hooks/misc/useSelectedOrganization' import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject' import { useCsvFileDrop } from 'hooks/ui/useCsvFileDrop' +import { Ref, forwardRef, memo, useMemo, useRef } from 'react' +import DataGrid, { CalculatedColumn, DataGridHandle } from 'react-data-grid' import { useTableEditorStateSnapshot } from 'state/table-editor' import { useTableEditorTableStateSnapshot } from 'state/table-editor-table' import { Button, cn } from 'ui' import { GenericSkeletonLoader } from 'ui-patterns/ShimmeringLoader' +import { ref as valtioRef } from 'valtio' + import type { GridProps, SupaRow } from '../../types' import { useOnRowsChange } from './Grid.utils' import { GridError } from './GridError' import RowRenderer from './RowRenderer' import { QueuedOperationType } from '@/state/table-editor-operation-queue.types' +import { ResponseError } from '@/types' const rowKeyGetter = (row: SupaRow) => { return row?.idx ?? -1 @@ -29,7 +30,7 @@ const rowKeyGetter = (row: SupaRow) => { interface IGrid extends GridProps { rows: SupaRow[] - error: Error | null + error: ResponseError | null isDisabled?: boolean isLoading: boolean isSuccess: boolean @@ -196,11 +197,10 @@ export const Grid = memo( {(rows ?? []).length === 0 && (
{ +import { HighCostError } from '@/components/ui/HighQueryCost' +import { COST_THRESHOLD_ERROR } from '@/data/sql/execute-sql-query' +import { ResponseError } from '@/types' + +export const GridError = ({ error }: { error?: ResponseError | null }) => { const { filters } = useTableFilter() const { sorts } = useTableSort() const snap = useTableEditorTableStateSnapshot() + if (!error) return null + const tableEntityType = snap.originalTable?.entity_type const isForeignTable = tableEntityType === ENTITY_TYPE.FOREIGN_TABLE @@ -26,7 +32,19 @@ export const GridError = ({ error }: { error?: any }) => { const isInvalidOrderingOperatorError = sorts.length > 0 && error?.message?.includes('identify an ordering operator') - if (isForeignTableMissingVaultKeyError) { + const isHighCostError = error?.message.includes(COST_THRESHOLD_ERROR) + + if (isHighCostError) { + return ( + + ) + } else if (isForeignTableMissingVaultKeyError) { return } else if (isInvalidSyntaxError) { return @@ -67,7 +85,7 @@ const ForeignTableMissingVaultKeyError = () => { ) } -const InvalidSyntaxError = ({ error }: { error?: any }) => { +const InvalidSyntaxError = ({ error }: { error: ResponseError }) => { const { onApplyFilters } = useTableFilter() return ( @@ -94,9 +112,9 @@ const InvalidSyntaxError = ({ error }: { error?: any }) => { ) } -const InvalidOrderingOperatorError = ({ error }: { error: any }) => { +const InvalidOrderingOperatorError = ({ error }: { error: ResponseError }) => { const { sorts, onApplySorts } = useTableSort() - const invalidDataType = (error?.message ?? '').split('type ').pop() + const invalidDataType = (error.message ?? '').split('type ').pop() ?? '' const formattedInvalidDataType = invalidDataType.includes('json') ? invalidDataType.toUpperCase() : invalidDataType @@ -127,13 +145,13 @@ const InvalidOrderingOperatorError = ({ error }: { error: any }) => { ) } -const GeneralError = ({ error }: { error: any }) => { +const GeneralError = ({ error }: { error: ResponseError }) => { const { filters } = useTableFilter() return ( {filters.length > 0 && ( diff --git a/apps/studio/components/grid/components/header/sort/SortRow.tsx b/apps/studio/components/grid/components/header/sort/SortRow.tsx index e13ca434808..a48ec064004 100644 --- a/apps/studio/components/grid/components/header/sort/SortRow.tsx +++ b/apps/studio/components/grid/components/header/sort/SortRow.tsx @@ -1,9 +1,8 @@ +import type { DragItem, Sort } from 'components/grid/types' import type { XYCoord } from 'dnd-core' import { Menu, X } from 'lucide-react' import { memo, useRef } from 'react' import { useDrag, useDrop } from 'react-dnd' - -import type { DragItem, Sort } from 'components/grid/types' import { useTableEditorTableStateSnapshot } from 'state/table-editor-table' import { Button, Switch } from 'ui' @@ -127,6 +126,7 @@ const SortRow = ({ index, columnName, sort, onDelete, onToggle, onDrag }: SortRo icon={} size="tiny" type="text" + className="w-7" onClick={() => onDelete(columnName)} />
diff --git a/apps/studio/components/interfaces/ExplainVisualizer/ExplainVisualizer.parser.ts b/apps/studio/components/interfaces/ExplainVisualizer/ExplainVisualizer.parser.ts index 35c5d4bea66..016972112f1 100644 --- a/apps/studio/components/interfaces/ExplainVisualizer/ExplainVisualizer.parser.ts +++ b/apps/studio/components/interfaces/ExplainVisualizer/ExplainVisualizer.parser.ts @@ -3,6 +3,7 @@ import type { ExplainNode, QueryPlanRow } from './ExplainVisualizer.types' export interface ExplainSummary { totalTime: number totalCost: number + maxCost: number hasSeqScan: boolean seqScanTables: string[] hasIndexScan: boolean @@ -246,6 +247,7 @@ export function calculateSummary(tree: ExplainNode[]): ExplainSummary { const stats: ExplainSummary = { totalTime: 0, totalCost: 0, + maxCost: 0, hasSeqScan: false, seqScanTables: [], hasIndexScan: false, @@ -256,7 +258,7 @@ export function calculateSummary(tree: ExplainNode[]): ExplainSummary { stats.totalTime = Math.max(stats.totalTime, node.actualTime.end) } if (node.cost) { - stats.totalCost = Math.max(stats.totalCost, node.cost.end) + stats.maxCost = Math.max(stats.maxCost, node.cost.end) } const op = node.operation.toLowerCase() if (op.includes('seq scan')) { @@ -270,6 +272,8 @@ export function calculateSummary(tree: ExplainNode[]): ExplainSummary { node.children.forEach(traverse) } tree.forEach(traverse) + + stats.totalCost = tree[0]?.cost?.end ?? 0 return stats } diff --git a/apps/studio/components/ui/HighQueryCost.tsx b/apps/studio/components/ui/HighQueryCost.tsx new file mode 100644 index 00000000000..e6a73ac0ec5 --- /dev/null +++ b/apps/studio/components/ui/HighQueryCost.tsx @@ -0,0 +1,102 @@ +import { + Button, + Dialog, + DialogClose, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogSection, + DialogSectionSeparator, + DialogTitle, + DialogTrigger, + Tooltip, + TooltipContent, + TooltipTrigger, +} from 'ui' +import { Admonition } from 'ui-patterns' + +import { DocsButton } from './DocsButton' +import { InlineLinkClassName } from './InlineLink' +import { DOCS_URL } from '@/lib/constants' +import { ResponseError } from '@/types' + +interface HighQueryCostErrorProps { + error: ResponseError + suggestions?: string[] +} + +export const HighCostError = ({ error, suggestions }: HighQueryCostErrorProps) => { + // [Joshen] The CTA could be to use a read replica to query or something? + return ( + + + + ) +} + +const HighQueryCostDialog = ({ error, suggestions = [] }: HighQueryCostErrorProps) => { + const metadata = error.metadata + + return ( + + + + + event.preventDefault()}> + + Estimated query cost exceeds safety thresholds + + Preventive measure to mitigate impacting the database + + + + +

+ The dashboard runs optimized SQL queries on your project’s database to load data for + this interface. +

+

+ However, the query was skipped as its{' '} + + estimated cost + +

Estimated cost: {metadata?.cost.toLocaleString()}

+

+ Determined via the EXPLAIN command +

+ + {' '} + is high and could place significant load on the database. +

+ {suggestions.length > 0 && ( +
+

You may check the following to ensure that the query cost is lower

+
    + {suggestions.map((x) => ( +
  • {x}
    • + ))} +
+
+ )} + + + + + + + + + + ) +} diff --git a/apps/studio/data/fetchers.ts b/apps/studio/data/fetchers.ts index 57ea115c056..968d46851b0 100644 --- a/apps/studio/data/fetchers.ts +++ b/apps/studio/data/fetchers.ts @@ -1,13 +1,15 @@ import * as Sentry from '@sentry/nextjs' - -import createClient from 'openapi-fetch' - import { DEFAULT_PLATFORM_APPLICATION_NAME } from '@supabase/pg-meta/src/constants' import { IS_PLATFORM, getAccessToken } from 'common' import { API_URL } from 'lib/constants' import { uuidv4 } from 'lib/helpers' +import createClient from 'openapi-fetch' import { ResponseError } from 'types' -import type { paths } from './api' // generated from openapi-typescript + +import type { paths } from './api' +import { ErrorMetadata } from '@/types/base' + +// generated from openapi-typescript const DEFAULT_HEADERS = { Accept: 'application/json' } @@ -164,9 +166,20 @@ export const handleError = (error: unknown, options: HandleErrorOptions = {}): n 'requestPathname' in error && typeof error.requestPathname === 'string' ? error.requestPathname : undefined + const metadata = + 'metadata' in error && typeof error.metadata === 'object' && !!error.metadata + ? (error.metadata as ErrorMetadata) + : undefined if (errorMessage) { - throw new ResponseError(errorMessage, errorCode, requestId, retryAfter, requestPathname) + throw new ResponseError( + errorMessage, + errorCode, + requestId, + retryAfter, + requestPathname, + metadata + ) } } diff --git a/apps/studio/data/sql/execute-sql-mutation.ts b/apps/studio/data/sql/execute-sql-mutation.ts index 763e4dadcaa..34df6a79449 100644 --- a/apps/studio/data/sql/execute-sql-mutation.ts +++ b/apps/studio/data/sql/execute-sql-mutation.ts @@ -1,11 +1,11 @@ import { useMutation, useQueryClient } from '@tanstack/react-query' -import { toast } from 'sonner' - import { useSendEventMutation } from 'data/telemetry/send-event-mutation' import { useSelectedOrganizationQuery } from 'hooks/misc/useSelectedOrganization' import { sqlEventParser } from 'lib/sql-event-parser' +import { toast } from 'sonner' import { UseCustomMutationOptions } from 'types' -import { executeSql, ExecuteSqlData, ExecuteSqlVariables } from './execute-sql-query' + +import { ExecuteSqlData, ExecuteSqlVariables, executeSql } from './execute-sql-query' // [Joshen] Intention is that we invalidate all database related keys whenever running a mutation related query // So we attempt to ignore all the non-related query keys. We could probably look into grouping our query keys better @@ -25,19 +25,24 @@ export type QueryResponseError = { severity: string } +type ExecuteSqlMutationVariables = ExecuteSqlVariables & { + autoLimit?: number + contextualInvalidation?: boolean +} + export const useExecuteSqlMutation = ({ onSuccess, onError, ...options }: Omit< - UseCustomMutationOptions, + UseCustomMutationOptions, 'mutationFn' > = {}) => { const queryClient = useQueryClient() const { mutate: sendEvent } = useSendEventMutation() const { data: org } = useSelectedOrganizationQuery() - return useMutation({ + return useMutation({ mutationFn: (args) => executeSql(args), async onSuccess(data, variables, context) { const { contextualInvalidation, sql, projectRef } = variables diff --git a/apps/studio/data/sql/execute-sql-query.ts b/apps/studio/data/sql/execute-sql-query.ts index cf58e425305..08fab08e683 100644 --- a/apps/studio/data/sql/execute-sql-query.ts +++ b/apps/studio/data/sql/execute-sql-query.ts @@ -1,6 +1,5 @@ -import { QueryKey, useQuery } from '@tanstack/react-query' - import { DEFAULT_PLATFORM_APPLICATION_NAME } from '@supabase/pg-meta/src/constants' +import { QueryKey, useQuery } from '@tanstack/react-query' import { handleError as handleErrorFetchers, post } from 'data/fetchers' import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject' import { MB, PROJECT_STATUS } from 'lib/constants' @@ -9,7 +8,21 @@ import { ROLE_IMPERSONATION_SQL_LINE_COUNT, } from 'lib/role-impersonation' import type { ResponseError, UseCustomQueryOptions } from 'types' + import { sqlKeys } from './keys' +import { + calculateSummary, + createNodeTree, +} from '@/components/interfaces/ExplainVisualizer/ExplainVisualizer.parser' + +/** + * [Joshen] Done a bit of stress testing and experimentation, tho we should still observe and tweak where necessary + * From what I understand a query cost of 100,000 is considered to be "heavy", and 1M is "potentially dangerous" + * Reckon we ensure that the dashboard just caps query costs at "heavy", so that it doesn't impact the DB for other queries + * (e.g from the user's application) + */ +const COST_THRESHOLD = 100_000 +export const COST_THRESHOLD_ERROR = 'Query cost exceeds threshold' export type ExecuteSqlVariables = { projectRef?: string @@ -18,9 +31,15 @@ export type ExecuteSqlVariables = { queryKey?: QueryKey handleError?: (error: ResponseError) => { result: any } isRoleImpersonationEnabled?: boolean + /** + * Disables transaction mode - should be used only for manual queries ran via the SQL Editor + * */ isStatementTimeoutDisabled?: boolean - autoLimit?: number - contextualInvalidation?: boolean + /** + * Runs an EXPLAIN before actually running the query, rejects the query if cost exceeds a threshold. + * Intended to be used for interfaces that heavily rely on queries on the DB + * */ + preflightCheck?: boolean } /** @@ -37,16 +56,8 @@ export async function executeSql( handleError, isRoleImpersonationEnabled = false, isStatementTimeoutDisabled = false, - }: Pick< - ExecuteSqlVariables, - | 'projectRef' - | 'connectionString' - | 'sql' - | 'queryKey' - | 'handleError' - | 'isRoleImpersonationEnabled' - | 'isStatementTimeoutDisabled' - >, + preflightCheck = false, + }: ExecuteSqlVariables, signal?: AbortSignal, headersInit?: HeadersInit, fetcherOverride?: (options: { @@ -76,26 +87,56 @@ export async function executeSql( error = result.error } } else { - const result = await post('/platform/pg-meta/{ref}/query', { + const options = { signal, + headers, params: { + path: { ref: projectRef }, header: { 'x-connection-encrypted': connectionString ?? '', 'x-pg-application-name': isStatementTimeoutDisabled ? 'supabase/dashboard-query-editor' : DEFAULT_PLATFORM_APPLICATION_NAME, }, - path: { ref: projectRef }, - // @ts-expect-error: This is just a client side thing to identify queries better - query: { - key: - queryKey - ?.filter((seg) => typeof seg === 'string' || typeof seg === 'number') - .join('-') ?? '', - }, }, + } + + if (preflightCheck) { + /** + * [Joshen] Note that I've intentionally omitted error handling here as I'm opting + * to NOT block the UI if the preflight check fails for any reason. + */ + + const { data: costCheck } = await post('/platform/pg-meta/{ref}/query', { + ...options, + body: { + query: `explain ${sql}`, + disable_statement_timeout: isStatementTimeoutDisabled, + }, + }) + const parsedTree = !!costCheck ? createNodeTree(costCheck) : undefined + const summary = !!parsedTree ? calculateSummary(parsedTree) : undefined + const cost = summary?.totalCost ?? 0 + + if (cost >= COST_THRESHOLD) { + return handleErrorFetchers({ + message: COST_THRESHOLD_ERROR, + code: cost, + metadata: { cost, sql }, + }) + } + } + + const key = + queryKey?.filter((seg) => typeof seg === 'string' || typeof seg === 'number').join('-') ?? '' + const result = await post('/platform/pg-meta/{ref}/query', { + ...options, body: { query: sql, disable_statement_timeout: isStatementTimeoutDisabled }, - headers, + params: { + ...options.params, + // @ts-expect-error: This is just a client side thing to identify queries better + query: { key }, + }, }) data = result.data diff --git a/apps/studio/data/table-rows/table-rows-query.ts b/apps/studio/data/table-rows/table-rows-query.ts index 9ddbc15fb1f..926edb3742d 100644 --- a/apps/studio/data/table-rows/table-rows-query.ts +++ b/apps/studio/data/table-rows/table-rows-query.ts @@ -1,7 +1,6 @@ import { Query, type QueryFilter } from '@supabase/pg-meta/src/query' import { getTableRowsSql } from '@supabase/pg-meta/src/query/table-row-query' -import { useQuery, useQueryClient, type QueryClient } from '@tanstack/react-query' - +import { type QueryClient, useQuery, useQueryClient } from '@tanstack/react-query' import { IS_PLATFORM } from 'common' import { parseSupaTable } from 'components/grid/SupabaseGrid.utils' import { Filter, Sort, SupaRow, SupaTable } from 'components/grid/types' @@ -15,6 +14,8 @@ import { } from 'lib/role-impersonation' import { isRoleImpersonationEnabled } from 'state/role-impersonation-state' import { ResponseError, UseCustomQueryOptions } from 'types' + +import { handleError } from '../fetchers' import { ExecuteSqlError, executeSql } from '../sql/execute-sql-query' import { tableRowKeys } from './keys' import { formatFilterValue } from './utils' @@ -374,6 +375,7 @@ export async function getTableRows( sql, queryKey: ['table-rows', table?.id], isRoleImpersonationEnabled: isRoleImpersonationEnabled(roleImpersonationState?.role), + preflightCheck: true, }, signal ) @@ -384,7 +386,7 @@ export async function getTableRows( return { rows } } catch (error) { - throw new Error(error instanceof Error ? error.message : 'Unknown error') + throw handleError(error) } } diff --git a/apps/studio/tests/features/explain-visualizer/ExplainVisualizer.parser.test.ts b/apps/studio/tests/features/explain-visualizer/ExplainVisualizer.parser.test.ts index 73cfe40d9b5..f42f0913363 100644 --- a/apps/studio/tests/features/explain-visualizer/ExplainVisualizer.parser.test.ts +++ b/apps/studio/tests/features/explain-visualizer/ExplainVisualizer.parser.test.ts @@ -1,15 +1,14 @@ -import { describe, test, expect } from 'vitest' import { - parseExplainOutput, - parseNodeDetails, calculateMaxCost, calculateSummary, - type ExplainSummary, + parseExplainOutput, + parseNodeDetails, } from 'components/interfaces/ExplainVisualizer/ExplainVisualizer.parser' import type { - QueryPlanRow, ExplainNode, + QueryPlanRow, } from 'components/interfaces/ExplainVisualizer/ExplainVisualizer.types' +import { describe, expect, test } from 'vitest' // Helper to create QueryPlanRow array from strings const toQueryPlanRows = (lines: string[]): QueryPlanRow[] => @@ -879,13 +878,14 @@ describe('calculateSummary', () => { expect(result).toEqual({ totalTime: 0, totalCost: 0, + maxCost: 0, hasSeqScan: false, seqScanTables: [], hasIndexScan: false, }) }) - test('calculates totalCost from cost.end', () => { + test('calculates totalCost from root node cost.end', () => { const tree: ExplainNode[] = [ { operation: 'Seq Scan', @@ -903,6 +903,47 @@ describe('calculateSummary', () => { expect(result.totalCost).toBe(45.5) }) + test('calculates maxCost from maximum cost across all nodes', () => { + const tree: ExplainNode[] = [ + { + operation: 'Limit', + details: '', + cost: { start: 0, end: 100 }, + rows: 10, + width: 36, + level: 0, + children: [ + { + operation: 'Sort', + details: '', + cost: { start: 0, end: 250 }, // This is the maximum + rows: 1000, + width: 36, + level: 1, + children: [ + { + operation: 'Seq Scan', + details: 'on users', + cost: { start: 0, end: 150 }, + rows: 1000, + width: 36, + level: 2, + children: [], + raw: '', + }, + ], + raw: '', + }, + ], + raw: '', + }, + ] + + const result = calculateSummary(tree) + expect(result.totalCost).toBe(100) // Root node cost + expect(result.maxCost).toBe(250) // Maximum across all nodes + }) + test('calculates totalTime from actualTime.end', () => { const tree: ExplainNode[] = [ { @@ -1090,7 +1131,8 @@ describe('calculateSummary', () => { ] const result = calculateSummary(tree) - expect(result.totalCost).toBe(35.8) + expect(result.totalCost).toBe(35.8) // Root node cost + expect(result.maxCost).toBe(35.8) // Maximum cost across all nodes (root is highest) expect(result.totalTime).toBe(2.345) expect(result.hasSeqScan).toBe(true) expect(result.hasIndexScan).toBe(true) diff --git a/apps/studio/types/base.ts b/apps/studio/types/base.ts index 81f5a5aff08..3981a57b14d 100644 --- a/apps/studio/types/base.ts +++ b/apps/studio/types/base.ts @@ -85,24 +85,36 @@ export interface ResponseFailure { export type SupaResponse = T | ResponseFailure +// [Joshen] Trialing returning metadata for the error object. It's meant to be generic +// but typed properly here, and we can create more types if needed with the | operator +type CostMetadata = { + cost: number + sql: string +} + +export type ErrorMetadata = CostMetadata + export class ResponseError extends Error { code?: number requestId?: string retryAfter?: number requestPathname?: string + metadata?: CostMetadata constructor( message: string | undefined, code?: number, requestId?: string, retryAfter?: number, - requestPathname?: string + requestPathname?: string, + metadata?: CostMetadata ) { super(message || 'API error happened while trying to communicate with the server.') this.code = code this.requestId = requestId this.retryAfter = retryAfter this.requestPathname = requestPathname + this.metadata = metadata } }