From 67d4fed40dc489c3a9d65f0df5d164be64cdaded Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Fri, 14 Aug 2026 11:29:39 +0700 Subject: [PATCH] Joshenlim/fe 4157 explorer migrate results component into explorer (#49066) ## Context Related to Notebooks/Explorers - this one's just shifting files from the SQLEditor into more generic folders from a file organization POV, such that files under the Explorer folder have no dependency on files within the SQLEditor folder Mainly - UtilityTabResults.utils: `getSqlErrorLines` - Moved into `data/sql/utils.ts` - SQLEditor.utils: `applyAutoLimit`, `getSqlErrorLines`, `trimTrailingSemicolons` - Moved into `data/sql/utils.ts` - SQLEditor/UtilityPanel: `ResultCell`, `Results`, `CellDetailPanel` - Moved into `components/ui/DataGridResults` - Also shifted corresponding tests over here - Also addressed some `any` type casts ## To test - Just need to ensure that the SQL Editor still works as expected ## Summary by CodeRabbit * **New Features** * Standardized query results across the Studio with a shared data grid. * Improved result-table formatting, column sizing, clipboard handling, and large-value display. * Added safer automatic row limits for eligible SQL queries. * Centralized SQL error display and formatting utilities. * **Refactor** * Improved type safety for query rows and cell values. * **Tests** * Added comprehensive coverage for result-grid and SQL utility behavior. --- .../components/grid/SupabaseGrid.utils.ts | 2 +- .../interfaces/Explorer/QueryEditor.tsx | 2 +- .../interfaces/Explorer/QueryResultTable.tsx | 6 +- .../Reports/ReportBlock/ReportBlock.tsx | 2 +- .../SQLEditor/SQLEditor.utils.test.ts | 162 ------------ .../interfaces/SQLEditor/SQLEditor.utils.ts | 69 +---- .../UtilityPanel/Results.utils.test.ts | 132 ---------- .../SQLEditor/UtilityPanel/Results.utils.ts | 39 --- .../UtilityPanel/UtilityTabResults.tsx | 6 +- .../UtilityTabResults.utils.test.ts | 80 ------ .../UtilityPanel/UtilityTabResults.utils.ts | 18 -- .../DataGridResults}/CellDetailPanel.tsx | 2 +- .../DataGridResults/DataGridResults.utils.ts | 40 +++ .../DataGridResults}/ResultCell.tsx | 2 +- .../__tests__/DataGridResults.test.tsx} | 39 ++- .../__tests__/DataGridResults.utils.test.ts | 138 ++++++++++ .../__tests__}/ResultCell.test.tsx | 2 +- .../DataGridResults/index.tsx} | 19 +- .../components/ui/EditorPanel/EditorPanel.tsx | 10 +- .../components/ui/QueryBlock/QueryBlock.tsx | 4 +- apps/studio/data/sql/__tests__/utils.test.ts | 241 ++++++++++++++++++ apps/studio/data/sql/utils.ts | 80 ++++++ 22 files changed, 560 insertions(+), 535 deletions(-) delete mode 100644 apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.test.ts delete mode 100644 apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.ts rename apps/studio/components/{interfaces/SQLEditor/UtilityPanel => ui/DataGridResults}/CellDetailPanel.tsx (99%) create mode 100644 apps/studio/components/ui/DataGridResults/DataGridResults.utils.ts rename apps/studio/components/{interfaces/SQLEditor/UtilityPanel => ui/DataGridResults}/ResultCell.tsx (95%) rename apps/studio/{tests/components/SQLEditor/Results.test.tsx => components/ui/DataGridResults/__tests__/DataGridResults.test.tsx} (54%) create mode 100644 apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.utils.test.ts rename apps/studio/{tests/components/SQLEditor => components/ui/DataGridResults/__tests__}/ResultCell.test.tsx (96%) rename apps/studio/components/{interfaces/SQLEditor/UtilityPanel/Results.tsx => ui/DataGridResults/index.tsx} (89%) create mode 100644 apps/studio/data/sql/__tests__/utils.test.ts create mode 100644 apps/studio/data/sql/utils.ts diff --git a/apps/studio/components/grid/SupabaseGrid.utils.ts b/apps/studio/components/grid/SupabaseGrid.utils.ts index 2818e55edbc..43b8ef31d1a 100644 --- a/apps/studio/components/grid/SupabaseGrid.utils.ts +++ b/apps/studio/components/grid/SupabaseGrid.utils.ts @@ -283,7 +283,7 @@ export function useSyncTableEditorStateFromLocalStorageWithUrl({ }, [urlParams, table, projectRef]) } -export const handleCellKeyDown = ( +export const handleCellKeyDown = = SupaRow>( args: CellKeyDownArgs, event: CellKeyboardEvent, context?: { diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor.tsx index 8c79ef00777..1a134b9dd7c 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor.tsx @@ -24,7 +24,6 @@ import { DisplaySettingsButton } from './QueryCell/DisplaySettingsButton' import { QueryResultChart } from './QueryCell/QueryResultChart' import { QueryResultTable } from './QueryResultTable' import { type QueryDisplay, type QueryResult } from './types' -import { applyAutoLimit } from '@/components/interfaces/SQLEditor/SQLEditor.utils' import { CodeEditor } from '@/components/ui/CodeEditor/CodeEditor' import { isValidConnString } from '@/data/fetchers' import { useExecuteLogsSqlMutation } from '@/data/logs/execute-logs-sql-mutation' @@ -36,6 +35,7 @@ import { } from '@/data/query-sources/query-source-registry' import { useReadReplicasQuery } from '@/data/read-replicas/replicas-query' import { useExecuteSqlMutation } from '@/data/sql/execute-sql-mutation' +import { applyAutoLimit } from '@/data/sql/utils' import { useLatest } from '@/hooks/misc/useLatest' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' diff --git a/apps/studio/components/interfaces/Explorer/QueryResultTable.tsx b/apps/studio/components/interfaces/Explorer/QueryResultTable.tsx index ba88cb1d274..abf405d0e78 100644 --- a/apps/studio/components/interfaces/Explorer/QueryResultTable.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryResultTable.tsx @@ -4,13 +4,13 @@ import { parseAsBoolean, useQueryState } from 'nuqs' import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' import { subscriptionHasHipaaAddon } from '../Billing/Subscription/Subscription.utils' -import { Results } from '../SQLEditor/UtilityPanel/Results' -import { getSqlErrorLines } from '../SQLEditor/UtilityPanel/UtilityTabResults.utils' import { type QueryResult } from './types' import { AiAssistantDropdown } from '@/components/ui/AiAssistantDropdown' import CopyButton from '@/components/ui/CopyButton' +import { DataGridResults } from '@/components/ui/DataGridResults' import { InlineLink, InlineLinkClassName } from '@/components/ui/InlineLink' import { useProjectSettingsV2Query } from '@/data/config/project-settings-v2-query' +import { getSqlErrorLines } from '@/data/sql/utils' import { useOrgSubscriptionQuery } from '@/data/subscriptions/org-subscription-query' import { useSelectedOrganizationQuery } from '@/hooks/misc/useSelectedOrganization' import { DOCS_URL } from '@/lib/constants' @@ -186,5 +186,5 @@ const QueryError = ({ // [Joshen] Eventually migrate the Results component here from SQL Editor const QueryResults = ({ rows }: { rows: NonNullable }) => { - return + return } diff --git a/apps/studio/components/interfaces/Reports/ReportBlock/ReportBlock.tsx b/apps/studio/components/interfaces/Reports/ReportBlock/ReportBlock.tsx index 2fea6cc8265..93d00b32219 100644 --- a/apps/studio/components/interfaces/Reports/ReportBlock/ReportBlock.tsx +++ b/apps/studio/components/interfaces/Reports/ReportBlock/ReportBlock.tsx @@ -5,7 +5,6 @@ import { X } from 'lucide-react' import { useEffect, useState } from 'react' import { toast } from 'sonner' -import { applyAutoLimit } from '../../SQLEditor/SQLEditor.utils' import { BURSTABLE_IO_METRIC_KEYS, DEPRECATED_REPORTS } from '../Reports.constants' import { ChartBlock } from './ChartBlock' import { DeprecatedChartBlock } from './DeprecatedChartBlock' @@ -20,6 +19,7 @@ import { useContentIdQuery } from '@/data/content/content-id-query' import { usePrimaryDatabase } from '@/data/read-replicas/replicas-query' import { executeSql } from '@/data/sql/execute-sql-mutation' import { sqlKeys } from '@/data/sql/keys' +import { applyAutoLimit } from '@/data/sql/utils' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' import { useDatabaseSelectorStateSnapshot } from '@/state/database-selector' import type { Dashboards, SqlSnippets } from '@/types' diff --git a/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts b/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts index d981a2da927..5689288a9ac 100644 --- a/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts +++ b/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts @@ -7,7 +7,6 @@ import { DiffType, type IStandaloneCodeEditor } from './SQLEditor.types' import { analyzeQueryIssues, appendEnableRLSStatements, - applyAutoLimit, assembleCompletionDiff, buildCompletionRequestBody, buildDebugChatArgs, @@ -30,7 +29,6 @@ import { resolveDiffKeyAction, shouldAutoGenerateTitle, sqlSourceToDialect, - trimTrailingSemicolons, } from './SQLEditor.utils' import type { DatabaseEventTrigger } from '@/data/database-event-triggers/database-event-triggers-query' import type { Database } from '@/data/read-replicas/replicas-query' @@ -49,166 +47,6 @@ const buildTrigger = (overrides: Partial = {}): DatabaseEv ...overrides, }) -describe('SQLEditor.utils.ts:trimTrailingSemicolons', () => { - test('removes a single trailing semicolon', () => { - const sql = safeSql`select * from countries;` - expect(trimTrailingSemicolons(sql)).toBe('select * from countries') - }) - test('removes multiple trailing semicolons', () => { - const sql = safeSql`select * from countries;;;;;;;` - expect(trimTrailingSemicolons(sql)).toBe('select * from countries') - }) - test('leaves a fragment with no trailing semicolon unchanged', () => { - const sql = safeSql`select * from countries` - expect(trimTrailingSemicolons(sql)).toBe('select * from countries') - }) - test('does not touch semicolons that are not trailing', () => { - const sql = safeSql`select 1; select 2` - expect(trimTrailingSemicolons(sql)).toBe('select 1; select 2') - }) -}) - -describe('SQLEditor.utils.ts:applyAutoLimit', () => { - test('Should return false if limit passed is <= 0', () => { - const sql = safeSql`select * from countries;` - const limit = -1 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return true if limit passed is > 0', () => { - const sql = safeSql`select * from countries;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(true) - }) - test('Should return false if query already has a limit', () => { - const sql = safeSql`select * from countries limit 10;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit (check for case-insensitiveness)', () => { - const sql = safeSql`SELECT * FROM countries LIMIT 10;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit with whitespace before the semi colon', () => { - const sql = safeSql`select * from countries limit 10 ;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit and offset', () => { - const sql = safeSql`select * from countries limit 10 offset 0;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit and offset with whitespace before the semi colon', () => { - const sql = safeSql`select * from countries limit 10 offset 0 ;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit and offset (flip order of limit and offset)', () => { - const sql = safeSql`select * from countries offset 0 limit 1;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query already has a limit, even if no value provided for limit', () => { - const sql = safeSql`select * from countries limit` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query uses `FETCH FIRST` instead of limit ', () => { - const sql = safeSql`select * from countries FETCH FIRST 5 rows only` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query uses `fetch first` instead of limit ', () => { - const sql = safeSql`select * from countries fetch first 5 rows only` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query uses `fetch first` (with random spaces) instead of limit ', () => { - const sql = safeSql`select * from countries FETCH FIRST 5 rows only` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query is not a select statement', () => { - const sql = safeSql`create table test (id int8 primary key, name varchar);` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if there are multiple queries I', () => { - const sql1 = safeSql`select * from countries; -select * from cities;` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql1, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if there are multiple queries II', () => { - const sql1 = safeSql`select * from countries; -select * from cities` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql1, limit) - expect(appendAutoLimit).toBe(false) - }) - // [Joshen] Opting to just avoid appending in this case to prevent making the logic overly complex atm - test('Should return false if query has with a comment I', () => { - const sql = safeSql`-- This is a comment -select * from cities` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - test('Should return false if query has with a comment II', () => { - const sql = safeSql`select * from cities --- This is a comment` - const limit = 100 - const { appendAutoLimit } = applyAutoLimit(sql, limit) - expect(appendAutoLimit).toBe(false) - }) - - // [Joshen] These will just need to test the cases when appendAutoLimit returns true then - test('Should add the limit param properly if query ends without a semi colon', () => { - const sql = safeSql`select * from countries` - const limit = 100 - const { sql: formattedSql } = applyAutoLimit(sql, limit) - expect(formattedSql).toBe('select * from countries limit 100;') - }) - test('Should add the limit param properly if query ends with a semi colon', () => { - const sql = safeSql`select * from countries;` - const limit = 100 - const { sql: formattedSql } = applyAutoLimit(sql, limit) - expect(formattedSql).toBe('select * from countries limit 100;') - }) - test('Should add the limit param properly if query ends with multiple semi colon', () => { - const sql = safeSql`select * from countries;;;;;;;` - const limit = 100 - const { sql: formattedSql } = applyAutoLimit(sql, limit) - expect(formattedSql).toBe('select * from countries limit 100;') - }) - test('Should not append a limit if query already has one with whitespace before the semi colon', () => { - const sql = safeSql`select * from countries limit 10 ;` - const limit = 100 - const { sql: formattedSql } = applyAutoLimit(sql, limit) - expect(formattedSql).toBe('select * from countries limit 10 ;') - }) - test('returns the SafeSqlFragment result unchanged when no limit is appended', () => { - const sql = safeSql`select * from countries limit 10;` - const { sql: formattedSql } = applyAutoLimit(sql, 100) - expect(formattedSql).toBe(sql) - }) -}) - describe('SQLEditor.utils.ts:shouldAutoGenerateTitle', () => { test('returns true when AI is enabled, the name is still the placeholder, and on platform', () => { expect( diff --git a/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts b/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts index 188c8c0fdce..040442d6147 100644 --- a/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts +++ b/apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts @@ -1,10 +1,4 @@ -import { - literal, - safeSql, - untrustedSql, - type SafeSqlFragment, - type UntrustedSqlFragment, -} from '@supabase/pg-meta' +import { untrustedSql, type SafeSqlFragment, type UntrustedSqlFragment } from '@supabase/pg-meta' import { TABLE_EVENT_ACTIONS } from 'common/telemetry-constants' import { isLogsSource, sqlSourceToFenceLanguage, type SqlSnippetSource } from './querySource' @@ -26,6 +20,7 @@ import type { SnippetWithContent } from '@/data/content/sql-folders-query' import type { DatabaseEventTrigger } from '@/data/database-event-triggers/database-event-triggers-query' import { untrustedLogSql, type UntrustedLogSqlFragment } from '@/data/logs/safe-analytics-sql' import type { Database } from '@/data/read-replicas/replicas-query' +import { applyAutoLimit } from '@/data/sql/utils' import { generateUuid } from '@/lib/api/snippets.browser' import { removeCommentsFromSql } from '@/lib/helpers' import { wrapWithRoleImpersonation } from '@/lib/role-impersonation' @@ -384,66 +379,6 @@ export const compareAsNewSnippet = (sqlDiff: ContentDiff) => { } } -/** - * Removes trailing `;` characters from a safe SQL fragment. Only ever removes - * existing terminators — never adds text — so the result is exactly as safe - * as the input; the brand carries over intentionally. This is the one place - * in the file allowed to reassert `SafeSqlFragment` on a derived string — - * every other function composes new fragments through `safeSql`/`literal`. - */ -export function trimTrailingSemicolons(sql: SafeSqlFragment): SafeSqlFragment { - return sql.replace(/;+\s*$/, '') as SafeSqlFragment -} - -// [Joshen] Just FYI as well the checks here on whether to append limit is quite restricted -// This is to prevent dashboard from accidentally appending limit to the end of a query -// thats not supposed to have any, since there's too many cases to cover. -// We can however look into making this logic better in the future -// i.e It's harder to append the limit param, than just leaving the query as it is -// Otherwise we'd need a full on parser to do this properly -// -// Only accepts `SafeSqlFragment`: this decides whether to build (and builds) -// a new SQL fragment that gets executed, so every caller — including ones -// that only want the `appendAutoLimit` flag for a display hint — must already -// hold safe SQL. Composes the ` limit N;` suffix through `safeSql`/`literal` -// rather than gluing raw template-literal text onto the fragment and casting -// the result, so the only new content this function ever stamps safe is an -// internally-generated integer literal, never arbitrary concatenated text. -export function applyAutoLimit( - sql: SafeSqlFragment, - limit: number = 0 -): { sql: SafeSqlFragment; appendAutoLimit: boolean } { - // Remove lines and whitespaces to use for checking - const cleanedSql = sql.trim().replaceAll('\n', ' ').replaceAll(/\s+/g, ' ') - - // Check how many queries - const regMatch = cleanedSql.matchAll(/[a-zA-Z]*[0-9]*[;]+/g) - const queries = new Array(...regMatch) - const indexSemiColon = cleanedSql.lastIndexOf(';') - const hasComments = cleanedSql.includes('--') - const hasMultipleQueries = - queries.length > 1 || (indexSemiColon > 0 && indexSemiColon !== cleanedSql.length - 1) - - // Check if need to auto limit rows - const appendAutoLimit = - limit > 0 && - !hasComments && - !hasMultipleQueries && - cleanedSql.toLowerCase().startsWith('select') && - !cleanedSql.toLowerCase().match(/fetch\s+first/i) && - !cleanedSql.match(/limit$/i) && - !cleanedSql.match(/limit;$/i) && - !cleanedSql.match(/limit [0-9]* offset [0-9]*\s*[;]?$/i) && - !cleanedSql.match(/limit [0-9]*\s*[;]?$/i) - - if (!appendAutoLimit) return { sql, appendAutoLimit: false } - - const core = cleanedSql.endsWith(';') ? trimTrailingSemicolons(sql) : sql - const suffixed = safeSql`${core} limit ${literal(limit)};` - - return { sql: suffixed, appendAutoLimit: true } -} - /** * Resolves the SQL to act on from the editor: the current selection if there is * one, otherwise the full editor contents, falling back to the snippet's stored diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.test.ts b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.test.ts index 88224d90764..f1fb5d068fa 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.test.ts +++ b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.test.ts @@ -1,100 +1,14 @@ import { describe, expect, it } from 'vitest' import { - calculateResultColumnWidth, convertResultsToCSV, convertResultsToJSON, convertResultsToMarkdown, - formatCellValue, - formatClipboardValue, formatResults, getResultsHeaders, - isLargeValue, } from './Results.utils' describe('Results.utils', () => { - describe('calculateResultColumnWidth', () => { - it('uses the minimum width when the column name and values are short', () => { - expect(calculateResultColumnWidth('id', [{ id: 1 }])).toBe(100) - }) - - it('accounts for a column name that is longer than its values', () => { - expect(calculateResultColumnWidth('source_campaign_id', [{ source_campaign_id: null }])).toBe( - 148.5 - ) - }) - - it('accounts for a value that is longer than the column name', () => { - expect(calculateResultColumnWidth('name', [{ name: 'a'.repeat(20) }])).toBe(165) - }) - - it('accounts for the formatted JSON representation of an object value', () => { - expect( - calculateResultColumnWidth('metadata', [{ metadata: { campaign: 'a'.repeat(20) } }]) - ).toBe(288.75) - }) - - it('accounts for the formatted JSON representation of an array value', () => { - expect(calculateResultColumnWidth('tags', [{ tags: ['a'.repeat(10), 'b'.repeat(10)] }])).toBe( - 222.75 - ) - }) - - it('caps the width when the column name exceeds the maximum', () => { - expect(calculateResultColumnWidth('a'.repeat(100), [])).toBe(500) - }) - - it('caps the width when a value exceeds the maximum', () => { - expect(calculateResultColumnWidth('value', [{ value: 'a'.repeat(100) }])).toBe(500) - }) - - it('uses the minimum width when there are no rows', () => { - expect(calculateResultColumnWidth('id', [])).toBe(100) - }) - }) - - describe('formatClipboardValue', () => { - it('returns empty string for null', () => { - expect(formatClipboardValue(null)).toBe('') - }) - - it('stringifies objects', () => { - expect(formatClipboardValue({ a: 1 })).toBe('{"a":1}') - }) - - it('stringifies arrays', () => { - expect(formatClipboardValue([1, 2])).toBe('[1,2]') - }) - - it('converts primitives to string', () => { - expect(formatClipboardValue('hello')).toBe('hello') - expect(formatClipboardValue(42)).toBe('42') - expect(formatClipboardValue(false)).toBe('false') - }) - }) - - describe('formatCellValue', () => { - it('returns NULL for null', () => { - expect(formatCellValue(null)).toBe('NULL') - }) - - it('returns strings as-is', () => { - expect(formatCellValue('hello')).toBe('hello') - }) - - it('stringifies objects', () => { - expect(formatCellValue({ a: 1 })).toBe('{"a":1}') - }) - - it('stringifies numbers', () => { - expect(formatCellValue(42)).toBe('42') - }) - - it('stringifies booleans', () => { - expect(formatCellValue(true)).toBe('true') - }) - }) - describe('formatResults', () => { it('should stringify object values', () => { const results = [{ id: 1, data: { nested: true } }] @@ -188,52 +102,6 @@ describe('Results.utils', () => { }) }) - describe('isLargeValue', () => { - it('returns false for null', () => { - expect(isLargeValue(null)).toBe(false) - }) - - it('returns false for undefined', () => { - expect(isLargeValue(undefined)).toBe(false) - }) - - it('returns false for an empty string', () => { - expect(isLargeValue('')).toBe(false) - }) - - it('returns false for a short string under the threshold', () => { - expect(isLargeValue('hello')).toBe(false) - }) - - it('returns false for a string at the 60-char boundary', () => { - expect(isLargeValue('a'.repeat(60))).toBe(false) - }) - - it('returns true for a string just over the 60-char threshold', () => { - expect(isLargeValue('a'.repeat(61))).toBe(true) - }) - - it('returns true for a short string containing a newline', () => { - expect(isLargeValue('hello\nworld')).toBe(true) - }) - - it('returns true for an object', () => { - expect(isLargeValue({ a: 1 })).toBe(true) - }) - - it('returns true for an array', () => { - expect(isLargeValue([1, 2, 3])).toBe(true) - }) - - it('returns false for a number', () => { - expect(isLargeValue(42)).toBe(false) - }) - - it('returns false for a boolean', () => { - expect(isLargeValue(true)).toBe(false) - }) - }) - describe('convertResultsToCSV', () => { it('should return undefined for empty results', () => { expect(convertResultsToCSV([])).toBeUndefined() diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.ts b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.ts index fb03dc9faf2..3791f6c9c61 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.ts +++ b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.utils.ts @@ -3,45 +3,6 @@ import Papa from 'papaparse' type ResultRow = Record -const ESTIMATED_CHARACTER_WIDTH = 8.25 -export const RESULT_COLUMN_MIN_WIDTH = 100 -const MAX_COLUMN_WIDTH = 500 - -export function calculateResultColumnWidth(columnName: string, rows: readonly ResultRow[]) { - const maxContentLength = rows.reduce( - (maxLength, row) => Math.max(maxLength, (formatCellValue(row[columnName]) ?? '').length), - columnName.length - ) - - return Math.min( - Math.max(maxContentLength * ESTIMATED_CHARACTER_WIDTH, RESULT_COLUMN_MIN_WIDTH), - MAX_COLUMN_WIDTH - ) -} - -export function formatClipboardValue(value: unknown) { - if (value === null) return '' - if (typeof value == 'object' || Array.isArray(value)) { - return JSON.stringify(value) - } - return String(value) -} - -export function formatCellValue(value: unknown) { - if (value === null) return 'NULL' - if (typeof value === 'string') return value - return JSON.stringify(value) -} - -const LARGE_VALUE_CHAR_THRESHOLD = 60 - -export function isLargeValue(value: unknown) { - if (value === null || value === undefined) return false - if (typeof value === 'object') return true - const str = String(value) - return str.length > LARGE_VALUE_CHAR_THRESHOLD || str.includes('\n') -} - export function formatResults( results: ResultRow[] ): Record[] { diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.tsx b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.tsx index 441af7bbca3..e2ebd2b4263 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.tsx +++ b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.tsx @@ -4,13 +4,13 @@ import { parseAsBoolean, useQueryState } from 'nuqs' import { forwardRef } from 'react' import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' -import { Results } from './Results' -import { getSqlErrorLines } from './UtilityTabResults.utils' import { subscriptionHasHipaaAddon } from '@/components/interfaces/Billing/Subscription/Subscription.utils' import { AiAssistantDropdown } from '@/components/ui/AiAssistantDropdown' import CopyButton from '@/components/ui/CopyButton' +import { DataGridResults } from '@/components/ui/DataGridResults' import { InlineLink, InlineLinkClassName } from '@/components/ui/InlineLink' import { useProjectSettingsV2Query } from '@/data/config/project-settings-v2-query' +import { getSqlErrorLines } from '@/data/sql/utils' import { useOrgSubscriptionQuery } from '@/data/subscriptions/org-subscription-query' import { useSelectedOrganizationQuery } from '@/hooks/misc/useSelectedOrganization' import { DOCS_URL } from '@/lib/constants' @@ -185,7 +185,7 @@ export const UtilityTabResults = forwardRef + return } ) diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.test.ts b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.test.ts deleted file mode 100644 index 779e4a635c5..00000000000 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { getSqlErrorLines } from './UtilityTabResults.utils' - -describe('getSqlErrorLines', () => { - it('returns formattedError lines when present', () => { - const lines = getSqlErrorLines({ - message: 'permission denied for table users', - formattedError: - 'ERROR: 42501: permission denied for table users\n' + - 'HINT: To grant access to anon on a specific table:\n' + - ' GRANT SELECT ON TABLE public.users TO anon;', - }) - - expect(lines).toEqual([ - 'ERROR: 42501: permission denied for table users', - 'HINT: To grant access to anon on a specific table:', - ' GRANT SELECT ON TABLE public.users TO anon;', - ]) - }) - - it('strips empty lines from formattedError', () => { - const lines = getSqlErrorLines({ - formattedError: 'ERROR: boom\n\nHINT: retry\n', - }) - - expect(lines).toEqual(['ERROR: boom', 'HINT: retry']) - }) - - it('falls back to message lines when formattedError is missing and message is multi-line', () => { - const lines = getSqlErrorLines({ - message: - 'ERROR: 42501: permission denied for table users\n' + - 'HINT: To grant access to anon on a specific table:\n' + - ' GRANT SELECT ON TABLE public.users TO anon;', - }) - - expect(lines).toEqual([ - 'ERROR: 42501: permission denied for table users', - 'HINT: To grant access to anon on a specific table:', - ' GRANT SELECT ON TABLE public.users TO anon;', - ]) - }) - - it('returns empty array for a single-line message so callers render the fallback', () => { - const lines = getSqlErrorLines({ message: 'permission denied for table users' }) - expect(lines).toEqual([]) - }) - - it('returns empty array when both fields are missing', () => { - expect(getSqlErrorLines({})).toEqual([]) - }) - - it('returns empty array when message is an empty string', () => { - expect(getSqlErrorLines({ message: '' })).toEqual([]) - }) - - it('returns empty array when message only contains whitespace newlines', () => { - // Only empty segments after filtering — treated as single-line - expect(getSqlErrorLines({ message: '\n\n' })).toEqual([]) - }) - - it('prefers formattedError even when message is also multi-line', () => { - const lines = getSqlErrorLines({ - message: 'message line 1\nmessage line 2', - formattedError: 'formatted line 1\nformatted line 2', - }) - - expect(lines).toEqual(['formatted line 1', 'formatted line 2']) - }) - - it('falls through to message when formattedError is empty string', () => { - const lines = getSqlErrorLines({ - message: 'ERROR: line 1\nHINT: line 2', - formattedError: '', - }) - - expect(lines).toEqual(['ERROR: line 1', 'HINT: line 2']) - }) -}) diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.ts b/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.ts deleted file mode 100644 index 583568ec02a..00000000000 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/UtilityTabResults.utils.ts +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Pick which lines to render for a SQL editor error. - * - * pg-meta returns `formattedError` with multi-line ERROR/HINT/LINE output from Postgres. - * Historically only `message` was reliably populated end-to-end, which is why the UI also - * falls back to splitting `message` on newlines — e.g. the enhanced permission-denied HINT - * added by supabase/postgres#2084 arrives in the message body on some paths. - * - * Returns an empty array when the error is single-line (message only) — callers fall back to - * a plain "Error: {message}" rendering in that case. - */ -export function getSqlErrorLines(error: { message?: string; formattedError?: string }): string[] { - const formattedLines = (error.formattedError?.split('\n') ?? []).filter((x) => x.length > 0) - if (formattedLines.length > 0) return formattedLines - - const messageLines = (error.message?.split('\n') ?? []).filter((x) => x.length > 0) - return messageLines.length > 1 ? messageLines : [] -} diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/CellDetailPanel.tsx b/apps/studio/components/ui/DataGridResults/CellDetailPanel.tsx similarity index 99% rename from apps/studio/components/interfaces/SQLEditor/UtilityPanel/CellDetailPanel.tsx rename to apps/studio/components/ui/DataGridResults/CellDetailPanel.tsx index 1ef52e82351..46be38eb901 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/CellDetailPanel.tsx +++ b/apps/studio/components/ui/DataGridResults/CellDetailPanel.tsx @@ -8,7 +8,7 @@ import { TwoOptionToggle } from '@/components/ui/TwoOptionToggle' interface CellDetailPanelProps { column: string - value: any + value: unknown visible: boolean onClose: () => void } diff --git a/apps/studio/components/ui/DataGridResults/DataGridResults.utils.ts b/apps/studio/components/ui/DataGridResults/DataGridResults.utils.ts new file mode 100644 index 00000000000..6aec73d2549 --- /dev/null +++ b/apps/studio/components/ui/DataGridResults/DataGridResults.utils.ts @@ -0,0 +1,40 @@ +export type ResultRow = Record + +const ESTIMATED_CHARACTER_WIDTH = 8.25 +export const RESULT_COLUMN_MIN_WIDTH = 100 +const MAX_COLUMN_WIDTH = 500 + +export function calculateResultColumnWidth(columnName: string, rows: readonly ResultRow[]) { + const maxContentLength = rows.reduce( + (maxLength, row) => Math.max(maxLength, (formatCellValue(row[columnName]) ?? '').length), + columnName.length + ) + + return Math.min( + Math.max(maxContentLength * ESTIMATED_CHARACTER_WIDTH, RESULT_COLUMN_MIN_WIDTH), + MAX_COLUMN_WIDTH + ) +} + +export function formatClipboardValue(value: unknown) { + if (value === null) return '' + if (typeof value == 'object' || Array.isArray(value)) { + return JSON.stringify(value) + } + return String(value) +} + +export function formatCellValue(value: unknown) { + if (value === null) return 'NULL' + if (typeof value === 'string') return value + return JSON.stringify(value) +} + +const LARGE_VALUE_CHAR_THRESHOLD = 60 + +export function isLargeValue(value: unknown) { + if (value === null || value === undefined) return false + if (typeof value === 'object') return true + const str = String(value) + return str.length > LARGE_VALUE_CHAR_THRESHOLD || str.includes('\n') +} diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/ResultCell.tsx b/apps/studio/components/ui/DataGridResults/ResultCell.tsx similarity index 95% rename from apps/studio/components/interfaces/SQLEditor/UtilityPanel/ResultCell.tsx rename to apps/studio/components/ui/DataGridResults/ResultCell.tsx index 6e89f2884d6..1fd909239fd 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/ResultCell.tsx +++ b/apps/studio/components/ui/DataGridResults/ResultCell.tsx @@ -1,7 +1,7 @@ import { Expand } from 'lucide-react' import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' -import { formatCellValue, isLargeValue } from './Results.utils' +import { formatCellValue, isLargeValue } from './DataGridResults.utils' interface ResultCellProps { column: string diff --git a/apps/studio/tests/components/SQLEditor/Results.test.tsx b/apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.test.tsx similarity index 54% rename from apps/studio/tests/components/SQLEditor/Results.test.tsx rename to apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.test.tsx index 9fb21a98dfe..d089d536050 100644 --- a/apps/studio/tests/components/SQLEditor/Results.test.tsx +++ b/apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.test.tsx @@ -1,7 +1,10 @@ import { screen } from '@testing-library/react' +import { type ComponentProps } from 'react' +import { type CalculatedColumn } from 'react-data-grid' import { expect, test, vi } from 'vitest' -import { Results } from '@/components/interfaces/SQLEditor/UtilityPanel/Results' +import { type ResultRow } from '../DataGridResults.utils' +import { DataGridResults as Results } from '../index' import { customRender as render } from '@/tests/lib/custom-render' let contextMenuMountCount = 0 @@ -10,7 +13,7 @@ vi.mock('ui', async () => { const actual = await vi.importActual('ui') return { ...actual, - ContextMenu: (props: any) => { + ContextMenu: (props: ComponentProps) => { contextMenuMountCount++ return }, @@ -18,20 +21,40 @@ vi.mock('ui', async () => { }) vi.mock('react-data-grid', () => ({ - default: ({ columns, rows }: any) => ( + default: ({ + columns, + rows, + }: { + columns: CalculatedColumn[] + rows: readonly ResultRow[] + }) => (
- {columns.map((col: any, colIdx: number) => ( + {columns.map((col, colIdx) => (
- {col.renderHeaderCell ? col.renderHeaderCell({}) : col.name} + {col.renderHeaderCell + ? col.renderHeaderCell({ + column: col, + sortDirection: undefined, + priority: undefined, + tabIndex: -1, + }) + : col.name}
))}
- {rows.map((row: any, rowIdx: number) => ( + {rows.map((row, rowIdx) => (
- {columns.map((col: any, colIdx: number) => ( + {columns.map((col, colIdx) => (
- {col.renderCell?.({ row, rowIdx, isCellSelected: false })} + {col.renderCell?.({ + column: col, + row, + rowIdx, + isCellEditable: false, + tabIndex: -1, + onRowChange: () => {}, + })}
))}
diff --git a/apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.utils.test.ts b/apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.utils.test.ts new file mode 100644 index 00000000000..ddbfd2a16b8 --- /dev/null +++ b/apps/studio/components/ui/DataGridResults/__tests__/DataGridResults.utils.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest' + +import { + calculateResultColumnWidth, + formatCellValue, + formatClipboardValue, + isLargeValue, +} from '../DataGridResults.utils' + +describe('Results.utils', () => { + describe('calculateResultColumnWidth', () => { + it('uses the minimum width when the column name and values are short', () => { + expect(calculateResultColumnWidth('id', [{ id: 1 }])).toBe(100) + }) + + it('accounts for a column name that is longer than its values', () => { + expect(calculateResultColumnWidth('source_campaign_id', [{ source_campaign_id: null }])).toBe( + 148.5 + ) + }) + + it('accounts for a value that is longer than the column name', () => { + expect(calculateResultColumnWidth('name', [{ name: 'a'.repeat(20) }])).toBe(165) + }) + + it('accounts for the formatted JSON representation of an object value', () => { + expect( + calculateResultColumnWidth('metadata', [{ metadata: { campaign: 'a'.repeat(20) } }]) + ).toBe(288.75) + }) + + it('accounts for the formatted JSON representation of an array value', () => { + expect(calculateResultColumnWidth('tags', [{ tags: ['a'.repeat(10), 'b'.repeat(10)] }])).toBe( + 222.75 + ) + }) + + it('caps the width when the column name exceeds the maximum', () => { + expect(calculateResultColumnWidth('a'.repeat(100), [])).toBe(500) + }) + + it('caps the width when a value exceeds the maximum', () => { + expect(calculateResultColumnWidth('value', [{ value: 'a'.repeat(100) }])).toBe(500) + }) + + it('uses the minimum width when there are no rows', () => { + expect(calculateResultColumnWidth('id', [])).toBe(100) + }) + }) + + describe('formatClipboardValue', () => { + it('returns empty string for null', () => { + expect(formatClipboardValue(null)).toBe('') + }) + + it('stringifies objects', () => { + expect(formatClipboardValue({ a: 1 })).toBe('{"a":1}') + }) + + it('stringifies arrays', () => { + expect(formatClipboardValue([1, 2])).toBe('[1,2]') + }) + + it('converts primitives to string', () => { + expect(formatClipboardValue('hello')).toBe('hello') + expect(formatClipboardValue(42)).toBe('42') + expect(formatClipboardValue(false)).toBe('false') + }) + }) + + describe('formatCellValue', () => { + it('returns NULL for null', () => { + expect(formatCellValue(null)).toBe('NULL') + }) + + it('returns strings as-is', () => { + expect(formatCellValue('hello')).toBe('hello') + }) + + it('stringifies objects', () => { + expect(formatCellValue({ a: 1 })).toBe('{"a":1}') + }) + + it('stringifies numbers', () => { + expect(formatCellValue(42)).toBe('42') + }) + + it('stringifies booleans', () => { + expect(formatCellValue(true)).toBe('true') + }) + }) + + describe('isLargeValue', () => { + it('returns false for null', () => { + expect(isLargeValue(null)).toBe(false) + }) + + it('returns false for undefined', () => { + expect(isLargeValue(undefined)).toBe(false) + }) + + it('returns false for an empty string', () => { + expect(isLargeValue('')).toBe(false) + }) + + it('returns false for a short string under the threshold', () => { + expect(isLargeValue('hello')).toBe(false) + }) + + it('returns false for a string at the 60-char boundary', () => { + expect(isLargeValue('a'.repeat(60))).toBe(false) + }) + + it('returns true for a string just over the 60-char threshold', () => { + expect(isLargeValue('a'.repeat(61))).toBe(true) + }) + + it('returns true for a short string containing a newline', () => { + expect(isLargeValue('hello\nworld')).toBe(true) + }) + + it('returns true for an object', () => { + expect(isLargeValue({ a: 1 })).toBe(true) + }) + + it('returns true for an array', () => { + expect(isLargeValue([1, 2, 3])).toBe(true) + }) + + it('returns false for a number', () => { + expect(isLargeValue(42)).toBe(false) + }) + + it('returns false for a boolean', () => { + expect(isLargeValue(true)).toBe(false) + }) + }) +}) diff --git a/apps/studio/tests/components/SQLEditor/ResultCell.test.tsx b/apps/studio/components/ui/DataGridResults/__tests__/ResultCell.test.tsx similarity index 96% rename from apps/studio/tests/components/SQLEditor/ResultCell.test.tsx rename to apps/studio/components/ui/DataGridResults/__tests__/ResultCell.test.tsx index 353c9b85877..39031a51209 100644 --- a/apps/studio/tests/components/SQLEditor/ResultCell.test.tsx +++ b/apps/studio/components/ui/DataGridResults/__tests__/ResultCell.test.tsx @@ -2,7 +2,7 @@ import { fireEvent, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { expect, test, vi } from 'vitest' -import { ResultCell } from '@/components/interfaces/SQLEditor/UtilityPanel/ResultCell' +import { ResultCell } from '../ResultCell' import { customRender as render } from '@/tests/lib/custom-render' const noop = () => {} diff --git a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.tsx b/apps/studio/components/ui/DataGridResults/index.tsx similarity index 89% rename from apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.tsx rename to apps/studio/components/ui/DataGridResults/index.tsx index ec9087f524e..6aa117000f4 100644 --- a/apps/studio/components/interfaces/SQLEditor/UtilityPanel/Results.tsx +++ b/apps/studio/components/ui/DataGridResults/index.tsx @@ -1,6 +1,6 @@ import { Copy, Expand } from 'lucide-react' import { useCallback, useMemo, useRef, useState } from 'react' -import DataGrid, { CalculatedColumn } from 'react-data-grid' +import DataGrid, { CalculatedColumn, RenderCellProps } from 'react-data-grid' import { ContextMenu, ContextMenuContent, @@ -10,20 +10,21 @@ import { } from 'ui' import { CellDetailPanel } from './CellDetailPanel' -import { ResultCell } from './ResultCell' import { calculateResultColumnWidth, formatClipboardValue, RESULT_COLUMN_MIN_WIDTH, -} from './Results.utils' + type ResultRow, +} from './DataGridResults.utils' +import { ResultCell } from './ResultCell' import { handleCellKeyDown } from '@/components/grid/SupabaseGrid.utils' -export const Results = ({ rows }: { rows: readonly any[] }) => { - const [expandedCell, setExpandedCell] = useState<{ column: string; value: any } | null>(null) - const contextMenuCellRef = useRef<{ column: string; value: any } | null>(null) +export const DataGridResults = ({ rows }: { rows: readonly ResultRow[] }) => { + const [expandedCell, setExpandedCell] = useState<{ column: string; value: unknown } | null>(null) + const contextMenuCellRef = useRef<{ column: string; value: unknown } | null>(null) const triggerRef = useRef(null) - const handleContextMenu = useCallback((e: React.MouseEvent, column: string, value: any) => { + const handleContextMenu = useCallback((e: React.MouseEvent, column: string, value: unknown) => { contextMenuCellRef.current = { column, value } if (triggerRef.current) { @@ -45,7 +46,7 @@ export const Results = ({ rows }: { rows: readonly any[] }) => { return
{name}
} - const columns: CalculatedColumn[] = useMemo( + const columns: CalculatedColumn[] = useMemo( () => Object.keys(rows?.[0] ?? []).map((key, idx) => { return { @@ -62,7 +63,7 @@ export const Results = ({ rows }: { rows: readonly any[] }) => { frozen: false, sortable: false, isLastFrozenColumn: false, - renderCell: ({ row }: { row: any }) => ( + renderCell: ({ row }: RenderCellProps) => ( { > {showResults && (
- +
)}
diff --git a/apps/studio/components/ui/QueryBlock/QueryBlock.tsx b/apps/studio/components/ui/QueryBlock/QueryBlock.tsx index 97907d4edb3..8fdbec57e03 100644 --- a/apps/studio/components/ui/QueryBlock/QueryBlock.tsx +++ b/apps/studio/components/ui/QueryBlock/QueryBlock.tsx @@ -9,6 +9,7 @@ import { ShimmeringLoader } from 'ui-patterns/ShimmeringLoader' import { ButtonTooltip } from '../ButtonTooltip' import { CHART_COLORS } from '../Charts/Charts.constants' import { PortalChartTooltip } from '../Charts/PortalChartTooltip' +import { DataGridResults } from '../DataGridResults' import { SqlWarningAdmonition } from '../SqlWarningAdmonition' import { BlockViewConfiguration } from './BlockViewConfiguration' import { EditQueryButton } from './EditQueryButton' @@ -21,7 +22,6 @@ import { } from './QueryBlock.utils' import { ReportBlockContainer } from '@/components/interfaces/Reports/ReportBlock/ReportBlockContainer' import { ChartConfig } from '@/components/interfaces/SQLEditor/UtilityPanel/ChartConfig' -import { Results } from '@/components/interfaces/SQLEditor/UtilityPanel/Results' export const DEFAULT_CHART_CONFIG: ChartConfig = { type: 'bar', @@ -391,7 +391,7 @@ export const QueryBlock = ({ 'flex flex-col flex-1 w-full overflow-auto overscroll-contain relative max-h-64' )} > - + {autoLimit && (

Limited to only 100 rows diff --git a/apps/studio/data/sql/__tests__/utils.test.ts b/apps/studio/data/sql/__tests__/utils.test.ts new file mode 100644 index 00000000000..2a7e0714cf8 --- /dev/null +++ b/apps/studio/data/sql/__tests__/utils.test.ts @@ -0,0 +1,241 @@ +import { safeSql } from '@supabase/pg-meta' +import { describe, expect, it, test } from 'vitest' + +import { applyAutoLimit, getSqlErrorLines, trimTrailingSemicolons } from '../utils' + +describe('getSqlErrorLines', () => { + it('returns formattedError lines when present', () => { + const lines = getSqlErrorLines({ + message: 'permission denied for table users', + formattedError: + 'ERROR: 42501: permission denied for table users\n' + + 'HINT: To grant access to anon on a specific table:\n' + + ' GRANT SELECT ON TABLE public.users TO anon;', + }) + + expect(lines).toEqual([ + 'ERROR: 42501: permission denied for table users', + 'HINT: To grant access to anon on a specific table:', + ' GRANT SELECT ON TABLE public.users TO anon;', + ]) + }) + + it('strips empty lines from formattedError', () => { + const lines = getSqlErrorLines({ + formattedError: 'ERROR: boom\n\nHINT: retry\n', + }) + + expect(lines).toEqual(['ERROR: boom', 'HINT: retry']) + }) + + it('falls back to message lines when formattedError is missing and message is multi-line', () => { + const lines = getSqlErrorLines({ + message: + 'ERROR: 42501: permission denied for table users\n' + + 'HINT: To grant access to anon on a specific table:\n' + + ' GRANT SELECT ON TABLE public.users TO anon;', + }) + + expect(lines).toEqual([ + 'ERROR: 42501: permission denied for table users', + 'HINT: To grant access to anon on a specific table:', + ' GRANT SELECT ON TABLE public.users TO anon;', + ]) + }) + + it('returns empty array for a single-line message so callers render the fallback', () => { + const lines = getSqlErrorLines({ message: 'permission denied for table users' }) + expect(lines).toEqual([]) + }) + + it('returns empty array when both fields are missing', () => { + expect(getSqlErrorLines({})).toEqual([]) + }) + + it('returns empty array when message is an empty string', () => { + expect(getSqlErrorLines({ message: '' })).toEqual([]) + }) + + it('returns empty array when message only contains whitespace newlines', () => { + // Only empty segments after filtering — treated as single-line + expect(getSqlErrorLines({ message: '\n\n' })).toEqual([]) + }) + + it('prefers formattedError even when message is also multi-line', () => { + const lines = getSqlErrorLines({ + message: 'message line 1\nmessage line 2', + formattedError: 'formatted line 1\nformatted line 2', + }) + + expect(lines).toEqual(['formatted line 1', 'formatted line 2']) + }) + + it('falls through to message when formattedError is empty string', () => { + const lines = getSqlErrorLines({ + message: 'ERROR: line 1\nHINT: line 2', + formattedError: '', + }) + + expect(lines).toEqual(['ERROR: line 1', 'HINT: line 2']) + }) +}) + +describe('trimTrailingSemicolons', () => { + test('removes a single trailing semicolon', () => { + const sql = safeSql`select * from countries;` + expect(trimTrailingSemicolons(sql)).toBe('select * from countries') + }) + test('removes multiple trailing semicolons', () => { + const sql = safeSql`select * from countries;;;;;;;` + expect(trimTrailingSemicolons(sql)).toBe('select * from countries') + }) + test('leaves a fragment with no trailing semicolon unchanged', () => { + const sql = safeSql`select * from countries` + expect(trimTrailingSemicolons(sql)).toBe('select * from countries') + }) + test('does not touch semicolons that are not trailing', () => { + const sql = safeSql`select 1; select 2` + expect(trimTrailingSemicolons(sql)).toBe('select 1; select 2') + }) +}) + +describe('applyAutoLimit', () => { + test('Should return false if limit passed is <= 0', () => { + const sql = safeSql`select * from countries;` + const limit = -1 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return true if limit passed is > 0', () => { + const sql = safeSql`select * from countries;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(true) + }) + test('Should return false if query already has a limit', () => { + const sql = safeSql`select * from countries limit 10;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit (check for case-insensitiveness)', () => { + const sql = safeSql`SELECT * FROM countries LIMIT 10;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit with whitespace before the semi colon', () => { + const sql = safeSql`select * from countries limit 10 ;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit and offset', () => { + const sql = safeSql`select * from countries limit 10 offset 0;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit and offset with whitespace before the semi colon', () => { + const sql = safeSql`select * from countries limit 10 offset 0 ;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit and offset (flip order of limit and offset)', () => { + const sql = safeSql`select * from countries offset 0 limit 1;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query already has a limit, even if no value provided for limit', () => { + const sql = safeSql`select * from countries limit` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query uses `FETCH FIRST` instead of limit ', () => { + const sql = safeSql`select * from countries FETCH FIRST 5 rows only` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query uses `fetch first` instead of limit ', () => { + const sql = safeSql`select * from countries fetch first 5 rows only` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query uses `fetch first` (with random spaces) instead of limit ', () => { + const sql = safeSql`select * from countries FETCH FIRST 5 rows only` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query is not a select statement', () => { + const sql = safeSql`create table test (id int8 primary key, name varchar);` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if there are multiple queries I', () => { + const sql1 = safeSql`select * from countries; +select * from cities;` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql1, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if there are multiple queries II', () => { + const sql1 = safeSql`select * from countries; +select * from cities` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql1, limit) + expect(appendAutoLimit).toBe(false) + }) + // [Joshen] Opting to just avoid appending in this case to prevent making the logic overly complex atm + test('Should return false if query has with a comment I', () => { + const sql = safeSql`-- This is a comment +select * from cities` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + test('Should return false if query has with a comment II', () => { + const sql = safeSql`select * from cities +-- This is a comment` + const limit = 100 + const { appendAutoLimit } = applyAutoLimit(sql, limit) + expect(appendAutoLimit).toBe(false) + }) + + // [Joshen] These will just need to test the cases when appendAutoLimit returns true then + test('Should add the limit param properly if query ends without a semi colon', () => { + const sql = safeSql`select * from countries` + const limit = 100 + const { sql: formattedSql } = applyAutoLimit(sql, limit) + expect(formattedSql).toBe('select * from countries limit 100;') + }) + test('Should add the limit param properly if query ends with a semi colon', () => { + const sql = safeSql`select * from countries;` + const limit = 100 + const { sql: formattedSql } = applyAutoLimit(sql, limit) + expect(formattedSql).toBe('select * from countries limit 100;') + }) + test('Should add the limit param properly if query ends with multiple semi colon', () => { + const sql = safeSql`select * from countries;;;;;;;` + const limit = 100 + const { sql: formattedSql } = applyAutoLimit(sql, limit) + expect(formattedSql).toBe('select * from countries limit 100;') + }) + test('Should not append a limit if query already has one with whitespace before the semi colon', () => { + const sql = safeSql`select * from countries limit 10 ;` + const limit = 100 + const { sql: formattedSql } = applyAutoLimit(sql, limit) + expect(formattedSql).toBe('select * from countries limit 10 ;') + }) + test('returns the SafeSqlFragment result unchanged when no limit is appended', () => { + const sql = safeSql`select * from countries limit 10;` + const { sql: formattedSql } = applyAutoLimit(sql, 100) + expect(formattedSql).toBe(sql) + }) +}) diff --git a/apps/studio/data/sql/utils.ts b/apps/studio/data/sql/utils.ts new file mode 100644 index 00000000000..1f23256fd1c --- /dev/null +++ b/apps/studio/data/sql/utils.ts @@ -0,0 +1,80 @@ +import { literal, safeSql, type SafeSqlFragment } from '@supabase/pg-meta' + +/** + * Pick which lines to render for a SQL editor error. + * + * pg-meta returns `formattedError` with multi-line ERROR/HINT/LINE output from Postgres. + * Historically only `message` was reliably populated end-to-end, which is why the UI also + * falls back to splitting `message` on newlines — e.g. the enhanced permission-denied HINT + * added by supabase/postgres#2084 arrives in the message body on some paths. + * + * Returns an empty array when the error is single-line (message only) — callers fall back to + * a plain "Error: {message}" rendering in that case. + */ +export function getSqlErrorLines(error: { message?: string; formattedError?: string }): string[] { + const formattedLines = (error.formattedError?.split('\n') ?? []).filter((x) => x.length > 0) + if (formattedLines.length > 0) return formattedLines + + const messageLines = (error.message?.split('\n') ?? []).filter((x) => x.length > 0) + return messageLines.length > 1 ? messageLines : [] +} + +/** + * Removes trailing `;` characters from a safe SQL fragment. Only ever removes + * existing terminators — never adds text — so the result is exactly as safe + * as the input; the brand carries over intentionally. This is the one place + * in the file allowed to reassert `SafeSqlFragment` on a derived string — + * every other function composes new fragments through `safeSql`/`literal`. + */ +export function trimTrailingSemicolons(sql: SafeSqlFragment): SafeSqlFragment { + return sql.replace(/;+\s*$/, '') as SafeSqlFragment +} + +// [Joshen] Just FYI as well the checks here on whether to append limit is quite restricted +// This is to prevent dashboard from accidentally appending limit to the end of a query +// thats not supposed to have any, since there's too many cases to cover. +// We can however look into making this logic better in the future +// i.e It's harder to append the limit param, than just leaving the query as it is +// Otherwise we'd need a full on parser to do this properly +// +// Only accepts `SafeSqlFragment`: this decides whether to build (and builds) +// a new SQL fragment that gets executed, so every caller — including ones +// that only want the `appendAutoLimit` flag for a display hint — must already +// hold safe SQL. Composes the ` limit N;` suffix through `safeSql`/`literal` +// rather than gluing raw template-literal text onto the fragment and casting +// the result, so the only new content this function ever stamps safe is an +// internally-generated integer literal, never arbitrary concatenated text. +export function applyAutoLimit( + sql: SafeSqlFragment, + limit: number = 0 +): { sql: SafeSqlFragment; appendAutoLimit: boolean } { + // Remove lines and whitespaces to use for checking + const cleanedSql = sql.trim().replaceAll('\n', ' ').replaceAll(/\s+/g, ' ') + + // Check how many queries + const regMatch = cleanedSql.matchAll(/[a-zA-Z]*[0-9]*[;]+/g) + const queries = new Array(...regMatch) + const indexSemiColon = cleanedSql.lastIndexOf(';') + const hasComments = cleanedSql.includes('--') + const hasMultipleQueries = + queries.length > 1 || (indexSemiColon > 0 && indexSemiColon !== cleanedSql.length - 1) + + // Check if need to auto limit rows + const appendAutoLimit = + limit > 0 && + !hasComments && + !hasMultipleQueries && + cleanedSql.toLowerCase().startsWith('select') && + !cleanedSql.toLowerCase().match(/fetch\s+first/i) && + !cleanedSql.match(/limit$/i) && + !cleanedSql.match(/limit;$/i) && + !cleanedSql.match(/limit [0-9]* offset [0-9]*\s*[;]?$/i) && + !cleanedSql.match(/limit [0-9]*\s*[;]?$/i) + + if (!appendAutoLimit) return { sql, appendAutoLimit: false } + + const core = cleanedSql.endsWith(';') ? trimTrailingSemicolons(sql) : sql + const suffixed = safeSql`${core} limit ${literal(limit)};` + + return { sql: suffixed, appendAutoLimit: true } +}