Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ export const SqlEditorView: React.FC = () => {

<label
className="flex items-center gap-1.5 text-[10px] font-semibold text-slate-500 ml-1"
title="When Safe mode is on, confirm runs that reference this many tables in one statement (0 = off). Suggests using a transaction."
title="When Safe mode is on, confirm writes that reference this many tables in one statement (0 = off). SELECT / JOIN reads skip this check. Suggests wrapping related writes in a transaction."
>
Tables≥
<input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface Props {
credentialCount: number;
/** sqlite / clickhouse targets that cannot execute writes. */
readonlyTargets?: ReadonlyWriteTarget[];
/** Statements that touch many tables (threshold from editor setting). */
/** Statements that touch many tables (writes only; SELECT/JOIN reads skip). */
multiTableStatements?: MultiTableWarn[];
/**
* When true (default), UPDATE/DELETE without WHERE require an explicit
Expand All @@ -38,7 +38,8 @@ const DML_LABEL: Record<string, string> = {
};

/**
* Safe-mode confirmation before writes / large multi-table runs.
* Safe-mode confirmation before writes / large multi-table *writes*.
* Read-only SELECT / JOIN queries never reach this dialog.
* Flags UPDATE/DELETE with no WHERE and asks for acknowledgment.
*/
export const WriteConfirmDialog: React.FC<Props> = ({
Expand Down
2 changes: 2 additions & 0 deletions apps/web/src/frontend/lib/sql-splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export {
extractTableAliases,
countReferencedTables,
referencedTableNames,
collectMultiTableWriteWarnings,
statementVerb,
isMutatingDmlStatement,
isInsertWriteStatement,
Expand Down Expand Up @@ -38,6 +39,7 @@ export type {
CodeCellOk,
CodeCellErr,
CodeCellResult,
MultiTableWriteWarning,
} from '@foxschema/sql';

export {
Expand Down
24 changes: 8 additions & 16 deletions apps/web/src/frontend/store/useSqlEditorStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import type { SavedConnectionSummary } from '../api/authApi';
import { resolveAppSecrets } from '../api/appSecretsApi';
import { loadSchema } from '../api/schemaApi';
import {
countReferencedTables,
collectMultiTableWriteWarnings,
isInsertWriteStatement,
isMutatingDmlStatement,
isPageableStatement,
isWriteStatement,
referencedTableNames,
splitSqlStatements,
} from '../lib/sql-splitter';
import type { CodeCellLast } from '../lib/codeCellExec';
Expand Down Expand Up @@ -383,7 +382,7 @@ interface SqlEditorState {
credentialCount: number;
/** Checked credentials whose dialect cannot execute writes. */
readonlyTargets: ReadonlyWriteTarget[];
/** Statements that touch many tables (threshold from setting). */
/** Write statements that meet the multi-table threshold (SELECT/JOIN reads omitted). */
multiTableStatements: Array<{ text: string; tableCount: number; tables: string[] }>;
/** When set, confirm resumes execute for only these credentials. */
connectionIds?: string[];
Expand All @@ -398,8 +397,9 @@ interface SqlEditorState {
*/
safeMode: boolean;
/**
* Prompt when a statement references this many (or more) tables.
* Suggests wrapping work in a transaction. `0` disables the check.
* Prompt when a *write* statement references this many (or more) tables.
* SELECT / JOIN reads skip this check. Suggests wrapping related writes in a
* transaction. `0` disables the check.
*/
multiTableConfirmThreshold: number;
/**
Expand Down Expand Up @@ -994,17 +994,9 @@ export const useSqlEditorStore = create<SqlEditorState>()(
// Insert-only writes skip Safe Mode; UPDATE/DELETE/MERGE and DDL still confirm.
const confirmWrites = writeStatements.filter((s) => !isInsertWriteStatement(s));
const mutatingDml = confirmWrites.filter((s) => isMutatingDmlStatement(s));
const threshold = multiTableConfirmThreshold;
const multiTableStatements =
safeMode && threshold > 0
? strippedForConfirm
.map((text) => {
const tableCount = countReferencedTables(text);
if (tableCount < threshold) return null;
return { text, tableCount, tables: referencedTableNames(text) };
})
.filter((x): x is { text: string; tableCount: number; tables: string[] } => x != null)
: [];
const multiTableStatements = safeMode
? collectMultiTableWriteWarnings(strippedForConfirm, multiTableConfirmThreshold)
: [];
const needsConfirm =
safeMode &&
!confirmedWrites &&
Expand Down
2 changes: 2 additions & 0 deletions packages/sql/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export {
extractTableAliases,
countReferencedTables,
referencedTableNames,
collectMultiTableWriteWarnings,
statementVerb,
isMutatingDmlStatement,
isInsertWriteStatement,
Expand All @@ -170,6 +171,7 @@ export type {
NodeCodeCellKind,
TsCodeCellKind,
CodeFenceRange,
MultiTableWriteWarning,
} from './modules/sql-splitter.js';
export {
parseFoxScript,
Expand Down
64 changes: 64 additions & 0 deletions packages/sql/src/modules/sql-splitter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
extractTableAliases,
countReferencedTables,
referencedTableNames,
collectMultiTableWriteWarnings,
isMutatingDmlStatement,
dmlLacksWhere,
parseCodeCell,
Expand Down Expand Up @@ -494,6 +495,69 @@ describe('countReferencedTables / referencedTableNames', () => {
});
});

describe('collectMultiTableWriteWarnings', () => {
const selectJoin = `
SELECT *
FROM users u
JOIN orders o ON o.user_id = u.id
JOIN items i ON i.order_id = o.id
JOIN payments p ON p.order_id = o.id
`;
const updateJoin = `
UPDATE users u
SET active = 0
FROM orders o
JOIN items i ON i.order_id = o.id
WHERE o.user_id = u.id
`;

it('does not warn on SELECT / JOIN reads that reference many tables', () => {
expect(collectMultiTableWriteWarnings([selectJoin], 3)).toEqual([]);
});

it('warns on UPDATE / DELETE / INSERT that meet the table threshold', () => {
const warnings = collectMultiTableWriteWarnings([updateJoin], 3);
expect(warnings).toHaveLength(1);
expect(warnings[0]!.tableCount).toBeGreaterThanOrEqual(3);
expect(warnings[0]!.tables.map((t) => t.toLowerCase())).toEqual(
expect.arrayContaining(['users', 'orders', 'items'])
);

const insert = collectMultiTableWriteWarnings(
['INSERT INTO dest SELECT * FROM a JOIN b ON 1=1 JOIN c ON 1=1'],
3
);
expect(insert).toHaveLength(1);
expect(insert[0]!.tableCount).toBeGreaterThanOrEqual(3);
});

it('still warns on SELECT … INTO (it is a write)', () => {
const warnings = collectMultiTableWriteWarnings(
['SELECT * INTO dest FROM a JOIN b ON 1=1 JOIN c ON 1=1'],
3
);
expect(warnings).toHaveLength(1);
});

it('ignores reads mixed into a batch and only reports writes', () => {
const warnings = collectMultiTableWriteWarnings([selectJoin, updateJoin, 'SELECT 1'], 3);
expect(warnings).toHaveLength(1);
expect(warnings[0]!.text).toBe(updateJoin);
});

it('does not warn on WITH … SELECT joins (read-only CTE wrapper)', () => {
const sql = `
WITH x AS (SELECT * FROM a JOIN b ON 1=1)
SELECT * FROM x JOIN c ON 1=1 JOIN d ON 1=1
`;
expect(collectMultiTableWriteWarnings([sql], 3)).toEqual([]);
});

it('is disabled when threshold is 0', () => {
expect(collectMultiTableWriteWarnings([updateJoin], 0)).toEqual([]);
});
});

describe('codeCellHasReturn with regex literals', () => {
it('sees a return after a regex that ends in an escaped slash', () => {
// `/\//` used to read as a `//` line comment, swallowing the return.
Expand Down
33 changes: 31 additions & 2 deletions packages/sql/src/modules/sql-splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1105,13 +1105,42 @@ export function dmlLacksWhere(text: string): boolean {

/**
* Distinct physical tables referenced by FROM/JOIN/UPDATE/INTO (aliases collapse
* to one table). Used for multi-table safety prompts (e.g. suggest a transaction
* when a statement touches many tables).
* to one table). Used for multi-table safety prompts on *writes* (e.g. suggest a
* transaction when an UPDATE/DELETE/INSERT touches many tables). Read-only
* SELECT / JOIN queries are not confirmation-worthy — see
* {@link collectMultiTableWriteWarnings}.
*/
export function countReferencedTables(sql: string): number {
return referencedTableNames(sql).length;
}

export type MultiTableWriteWarning = {
text: string;
tableCount: number;
tables: string[];
};

/**
* Safe-mode multi-table confirmations apply only to write statements.
* A SELECT that joins many tables is a read; prompting for a transaction
* there is noise. INSERT/UPDATE/DELETE/DDL (including `SELECT … INTO`)
* that meet `threshold` still warn. `threshold <= 0` disables the check.
*/
export function collectMultiTableWriteWarnings(
statements: string[],
threshold: number
): MultiTableWriteWarning[] {
if (threshold <= 0) return [];
const out: MultiTableWriteWarning[] = [];
for (const text of statements) {
if (!isWriteStatement(text)) continue;
const tables = referencedTableNames(text);
if (tables.length < threshold) continue;
out.push({ text, tableCount: tables.length, tables });
}
return out;
}

/** Unique bare/qualified table names from {@link extractTableAliases} values. */
export function referencedTableNames(sql: string): string[] {
const map = extractTableAliases(sql);
Expand Down
Loading