diff --git a/apps/e2e/src/tests/schema-history.test.ts b/apps/e2e/src/tests/schema-history.test.ts index 61ed004e..21b2b37e 100644 --- a/apps/e2e/src/tests/schema-history.test.ts +++ b/apps/e2e/src/tests/schema-history.test.ts @@ -120,11 +120,13 @@ describe.skipIf(!ready)('Schema Sync · History (SQLite)', () => { expect(await driver.locator('[data-testid="lokee-inspector-script-diff"]').isVisible()).toBe(true); }); - it('defaults to tables, views, functions, procedures and changes only', async () => { + it('defaults to containers and changes only', async () => { + // Functions and procedures are still in the default set, but this seed has + // neither, so their boxes are not offered at all — the sibling test below + // covers that. Asserting `isChecked()` on a control that is deliberately + // absent just hangs until the timeout. expect(await driver.locator('[data-testid="lokee-rf-type-table"]').isChecked()).toBe(true); expect(await driver.locator('[data-testid="lokee-rf-type-view"]').isChecked()).toBe(true); - expect(await driver.locator('[data-testid="lokee-rf-type-function"]').isChecked()).toBe(true); - expect(await driver.locator('[data-testid="lokee-rf-type-procedure"]').isChecked()).toBe(true); expect(await driver.locator('[data-testid="lokee-rf-type-column"]').isChecked()).toBe(false); expect(await driver.locator('[data-testid="lokee-rf-type-index"]').isChecked()).toBe(false); expect(await driver.locator('[data-testid="lokee-rf-changes-only"]').isChecked()).toBe(true); diff --git a/apps/web/src/backend/api/index-fragmentation.ts b/apps/web/src/backend/api/index-fragmentation.ts index 6d1824ba..fc4002c4 100644 --- a/apps/web/src/backend/api/index-fragmentation.ts +++ b/apps/web/src/backend/api/index-fragmentation.ts @@ -104,6 +104,26 @@ async function runProbe( * Probe one table. Returns either a result or a structured failure * (caller maps `status` onto the HTTP response). */ +/** + * Turn a driver error into something the reader can act on. + * + * The physical probes lean on optional server features — pgstatindex comes from + * the pgstattuple extension, and it is not installed by default. "function + * pgstatindex(regclass) does not exist" is true and useless; the reader wants + * the one line that fixes it. + */ +export function explainFragmentationError(message: string, dialect: string): string { + const missingFn = /function\s+pgstat(index|tuple)[^)]*\)?\s+does not exist/i.test(message); + if (missingFn) { + return ( + `Index fragmentation on ${dialect} needs the pgstattuple extension, which is not ` + + `installed on this server. A superuser can add it with: CREATE EXTENSION pgstattuple; ` + + `— or supply your own query below. (${message})` + ); + } + return message; +} + export async function probeTableFragmentation(opts: { dialect: string; option: ConnectionOptions; @@ -239,8 +259,9 @@ export async function probeTableFragmentation(opts: { ); return { ok: true, value }; } catch (defaultErr: unknown) { - const defaultMessage = + const rawDefaultMessage = defaultErr instanceof Error ? defaultErr.message : 'Default fragmentation query failed'; + const defaultMessage = explainFragmentationError(rawDefaultMessage, dialect); if (!customSql) { return { ok: false, diff --git a/apps/web/src/frontend/components/ConnectionModal.tsx b/apps/web/src/frontend/components/ConnectionModal.tsx index f60529cb..cea5c9cf 100644 --- a/apps/web/src/frontend/components/ConnectionModal.tsx +++ b/apps/web/src/frontend/components/ConnectionModal.tsx @@ -1,7 +1,15 @@ import React, { useState, useEffect } from "react"; import { createPortal } from "react-dom"; import { X, CheckCircle, AlertTriangle, FolderOpen, Loader2, ListTree, Download } from "lucide-react"; -import { type ConnectionOptions, type Dialect, buildConnectionString, DEFAULT_PORTS, getProviderSettings, PROVIDER_SETTINGS } from '../lib/provider-settings'; +import { + type ConnectionOptions, + type Dialect, + buildConnectionString, + DEFAULT_PORTS, + getProviderSettings, + isFileDialect as isFileDialectOf, + PROVIDER_SETTINGS, +} from '../lib/provider-settings'; import type { DriverInfo } from '../lib/types'; import { fetchSchemaList, checkDriver as apiCheckDriver, installDriver as apiInstallDriver } from "../api/schemaApi"; import { PasswordInput } from './PasswordInput'; @@ -74,7 +82,7 @@ export const ConnectionModal: React.FC = ({ * password and SSL have no meaning for them, and showing the boxes anyway * has people filling in `localhost` and wondering why it changes nothing. */ - const isFileDialect = selDialect === 'sqlite' || selDialect === 'duckdb'; + const isFileDialect = isFileDialectOf(selDialect); const [browsing, setBrowsing] = useState(false); const [driverInfo, setDriverInfo] = useState(null); diff --git a/apps/web/src/frontend/components/TopToolbar.tsx b/apps/web/src/frontend/components/TopToolbar.tsx index 0a0c4b7b..6d66143d 100644 --- a/apps/web/src/frontend/components/TopToolbar.tsx +++ b/apps/web/src/frontend/components/TopToolbar.tsx @@ -10,6 +10,7 @@ import { CredentialManager } from './CredentialManager'; import { MigrationHistory } from './MigrationHistory'; import { TYPE_META, TYPE_ORDER } from './SchemaTreePanel'; import type { DbObjectType } from '../lib/types'; +import { dialectUsesPassword } from '../lib/provider-settings'; import { ConnectionModal } from './ConnectionModal'; import { PasswordInput } from './PasswordInput'; import { useAuthStore } from '../store/authStore'; @@ -69,7 +70,9 @@ export const TopToolbar: React.FC = () => { const selectSavedConnection = (side: 'source' | 'target', id: string) => { const conn = connections.find((c) => c.id === id); - if (conn && !conn.hasPassword) { + // A file dialect has no password to be missing. Prompting for one left the + // picker snapping back to "— Saved —" and no target selected at all. + if (conn && !conn.hasPassword && dialectUsesPassword(conn.dialect)) { // Reuse a password already typed this session (SQL Editor or prior Sync pick). const cfg = side === 'source' ? sourceConfig : targetConfig; const existing = diff --git a/apps/web/src/frontend/lib/provider-settings.ts b/apps/web/src/frontend/lib/provider-settings.ts index e599095b..8490a69e 100644 --- a/apps/web/src/frontend/lib/provider-settings.ts +++ b/apps/web/src/frontend/lib/provider-settings.ts @@ -230,6 +230,24 @@ export const PROVIDER_SETTINGS: Record = { mongodb: mongodbSettings, }; +/** + * Dialects that are a file on disk rather than a server. + * + * They have no host, port, user or password, so anything that asks for one has + * to know not to. Selecting a saved SQLite connection used to open a password + * prompt — the credential form had stopped offering a password to store, and + * the picker still treated "no stored password" as "ask the user for one", + * which snapped the selection back and left no target set. + */ +export function isFileDialect(dialect: string): boolean { + return dialect === 'sqlite' || dialect === 'duckdb'; +} + +/** True when a connection of this dialect can meaningfully carry a password. */ +export function dialectUsesPassword(dialect: string): boolean { + return !isFileDialect(dialect); +} + export const DEFAULT_PORTS: Record = Object.fromEntries( Object.values(PROVIDER_SETTINGS).map((s) => [s.dialect, s.defaultPort]) ); diff --git a/packages/sql/src/modules/dialect-dba-utilities.test.ts b/packages/sql/src/modules/dialect-dba-utilities.test.ts index 5eb4c214..7a01e72b 100644 --- a/packages/sql/src/modules/dialect-dba-utilities.test.ts +++ b/packages/sql/src/modules/dialect-dba-utilities.test.ts @@ -126,3 +126,54 @@ describe('DB2 session query columns', () => { expect(sql).toContain('CURRENT SERVER AS database_name'); }); }); + +describe('MariaDB is not a MySQL alias for these probes', () => { + /** + * Both bugs verified against MariaDB 11.8: + * + * SELECT @@innodb_buffer_pool_instances; + * ERROR 1193 (HY000): Unknown system variable + * SELECT VARIABLE_VALUE FROM performance_schema.global_status WHERE ... + * (empty — performance_schema is off by default) + * SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE ... + * 245412 + * + * The first killed System info outright. The second was worse: no error, a + * blank connection count in the pool panel. + */ + const sqlFor = (kind: 'pool' | 'sessions' | 'system' | 'sizes') => { + const q = buildDbaUtilityQuery({ dialect: 'mariadb', kind, schema: 'foxdb' }); + if ('error' in q) throw new Error(`${kind}: ${q.error}`); + return q.sql; + }; + + it('does not reference a system variable MariaDB removed in 10.5', () => { + expect(sqlFor('system')).not.toMatch(/innodb_buffer_pool_instances/); + }); + + it('reads status from information_schema, which MariaDB populates by default', () => { + for (const kind of ['pool', 'system'] as const) { + expect(sqlFor(kind), kind).toMatch(/information_schema\.GLOBAL_STATUS/i); + expect(sqlFor(kind), kind).not.toMatch(/performance_schema\.global_status/i); + } + }); + + it('still answers every utility kind rather than erroring on an unknown family', () => { + // Splitting mariadb out of the mysql family is what would break these. + for (const kind of ['pool', 'sessions', 'system', 'sizes'] as const) { + expect(sqlFor(kind).length, kind).toBeGreaterThan(0); + } + }); + + it('keeps buffer pool size and uptime, which MariaDB does have', () => { + const sql = sqlFor('system'); + expect(sql).toMatch(/innodb_buffer_pool_size/); + expect(sql).toMatch(/'Uptime'/); + }); + + it('leaves MySQL itself on the performance_schema path', () => { + const q = buildDbaUtilityQuery({ dialect: 'mysql', kind: 'pool' }); + if ('error' in q) throw new Error(q.error); + expect(q.sql).toMatch(/performance_schema\.global_status/i); + }); +}); diff --git a/packages/sql/src/modules/dialect-dba-utilities.ts b/packages/sql/src/modules/dialect-dba-utilities.ts index 45dbc63e..3bf84534 100644 --- a/packages/sql/src/modules/dialect-dba-utilities.ts +++ b/packages/sql/src/modules/dialect-dba-utilities.ts @@ -84,7 +84,18 @@ const UNSUPPORTED: DbaUtilitySupport = { function family(dialect: string): string { const d = (dialect || '').toLowerCase(); if (d === 'azuresql') return 'sqlserver'; - if (d === 'mariadb' || d === 'tidb') return 'mysql'; + /** + * MariaDB is its own family here, not a MySQL alias. + * + * Two of these probes are wrong on it. `@@innodb_buffer_pool_instances` was + * removed in MariaDB 10.5, so System info died on `Unknown system variable` + * before returning anything. And `performance_schema` is off by default, so + * the status lookups the pool probe leans on came back *empty* — no error, a + * blank connection count, which is worse. MariaDB keeps the same figures in + * `information_schema.GLOBAL_STATUS`. + */ + if (d === 'mariadb') return 'mariadb'; + if (d === 'tidb') return 'mysql'; if (d === 'cockroachdb' || d === 'yugabytedb') return 'postgres'; return d; } @@ -284,6 +295,20 @@ SELECT (SELECT VARIABLE_VALUE FROM performance_schema.global_status WHERE VARIABLE_NAME = 'Threads_running' LIMIT 1) AS active_connections, (SELECT VARIABLE_VALUE FROM performance_schema.global_status WHERE VARIABLE_NAME = 'Threads_cached' LIMIT 1) AS available_connections, (SELECT VARIABLE_VALUE FROM performance_schema.global_status WHERE VARIABLE_NAME = 'Connection_errors_max_connections' LIMIT 1) AS wait_count +`.trim(), + }; + } + if (f === 'mariadb') { + return { + mode: asMode(mode), + params: [], + sql: ` +SELECT + CAST(@@max_connections AS SIGNED) AS max_connections, + (SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Threads_connected' LIMIT 1) AS current_connections, + (SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Threads_running' LIMIT 1) AS active_connections, + (SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Threads_cached' LIMIT 1) AS available_connections, + (SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Connection_errors_max_connections' LIMIT 1) AS wait_count `.trim(), }; } @@ -380,7 +405,7 @@ LIMIT 500 `.trim(), }; } - if (f === 'mysql') { + if (f === 'mysql' || f === 'mariadb') { return { mode: asMode(mode), params: [], @@ -557,6 +582,25 @@ SELECT NULL::bigint AS storage_available_bytes, EXTRACT(EPOCH FROM (now() - pg_postmaster_start_time()))::bigint AS uptime_seconds, version() AS server_version +`.trim(), + }; + } + if (f === 'mariadb') { + return { + mode: asMode(mode), + params: [], + sql: ` +SELECT + NULL AS cpu_count, + NULL AS cpu_usage_percent, + CAST(@@innodb_buffer_pool_size AS SIGNED) AS memory_total_bytes, + NULL AS memory_used_bytes, + NULL AS memory_available_bytes, + NULL AS storage_total_bytes, + (SELECT SUM(DATA_LENGTH + INDEX_LENGTH) FROM information_schema.TABLES WHERE TABLE_SCHEMA = DATABASE()) AS storage_used_bytes, + NULL AS storage_available_bytes, + (SELECT VARIABLE_VALUE FROM information_schema.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Uptime' LIMIT 1) AS uptime_seconds, + VERSION() AS server_version `.trim(), }; } @@ -729,7 +773,7 @@ LIMIT 1000 `.trim(), }; } - if (f === 'mysql') { + if (f === 'mysql' || f === 'mariadb') { return { mode: asMode(mode), params: [], diff --git a/packages/sql/src/modules/dialect-index-fragmentation.test.ts b/packages/sql/src/modules/dialect-index-fragmentation.test.ts index 793ea2da..2eb3816b 100644 --- a/packages/sql/src/modules/dialect-index-fragmentation.test.ts +++ b/packages/sql/src/modules/dialect-index-fragmentation.test.ts @@ -68,7 +68,20 @@ describe('buildIndexFragmentationQuery', () => { expect(q.sql).toMatch(/OBJECT_ID\(\?\)/); }); - it('builds Postgres pgstattuple probe', () => { + it('asks pgstatindex for leaf_fragmentation, not pgstattuple', () => { + /** + * The bug this pins, verified against PostgreSQL 17: + * + * SELECT leaf_fragmentation FROM pgstattuple('i'::regclass::oid) + * ERROR: column "leaf_fragmentation" does not exist + * SELECT leaf_fragmentation FROM pgstatindex('i') + * 0 + * + * `pgstattuple` reports *table* statistics. Naming it here failed twice + * over: without the extension Postgres says the function does not exist — + * which is what users hit — and installing the extension, the obvious fix, + * then failed on the missing column. The probe could never have worked. + */ const q = buildIndexFragmentationQuery({ dialect: 'postgres', table: 'public.users', @@ -76,7 +89,17 @@ describe('buildIndexFragmentationQuery', () => { expect('error' in q).toBe(false); if ('error' in q) return; expect(q.params).toEqual(['public', 'users']); - expect(q.sql).toMatch(/pgstattuple/); + expect(q.sql).toMatch(/pgstatindex\(/); + expect(q.sql).not.toMatch(/FROM pgstattuple\(/); + }); + + it('does not let an unmeasured index reach the grid as NaN', () => { + // pgstatindex returns NaN for an index with no leaf pages yet. That is + // "nothing measured", not a number, and "NaN%" in a column reads as a bug. + const q = buildIndexFragmentationQuery({ dialect: 'postgres', table: 'public.users' }); + if ('error' in q) throw new Error(q.error); + expect(q.sql).toMatch(/NULLIF\(/); + expect(q.sql).toMatch(/'NaN'::float8/); }); it('builds probes for formerly unsupported dialects', () => { diff --git a/packages/sql/src/modules/dialect-index-fragmentation.ts b/packages/sql/src/modules/dialect-index-fragmentation.ts index bf56b9ca..acb49232 100644 --- a/packages/sql/src/modules/dialect-index-fragmentation.ts +++ b/packages/sql/src/modules/dialect-index-fragmentation.ts @@ -8,7 +8,11 @@ * * Quality ladder: * - SQL Server / Azure SQL: physical % via dm_db_index_physical_stats - * - PostgreSQL family: leaf_fragmentation via pgstattuple (extension; may fail) + * - PostgreSQL family: leaf_fragmentation via pgstatindex, from the pgstattuple + * extension. `pgstattuple` itself reports *table* statistics and has no + * leaf_fragmentation column — naming it here failed twice over: without the + * extension Postgres says the function does not exist, and with it installed + * the column does not exist either. * - MySQL family: table-level DATA_FREE ratio applied per index (estimate) * - DB2: empty-leaf ratio from SYSCAT.INDEXES (estimate) * - Oracle: weak estimate from ALL_INDEXES stats (prefer custom ANALYZE) @@ -68,21 +72,21 @@ const SUPPORT: Record = { mode: 'physical', query: true, defrag: true, - hint: 'PostgreSQL: leaf_fragmentation via pgstattuple (requires extension). Falls back to custom SQL on failure.', + hint: 'PostgreSQL: leaf_fragmentation via pgstatindex, from the pgstattuple extension. Install it with CREATE EXTENSION pgstattuple; falls back to custom SQL on failure.', customSqlHint: CUSTOM_HINT, }, cockroachdb: { mode: 'estimated', query: true, defrag: true, - hint: 'CockroachDB: tries pgstattuple-compatible probe; often needs custom SQL.', + hint: 'CockroachDB: tries the pgstatindex probe; often needs custom SQL.', customSqlHint: CUSTOM_HINT, }, yugabytedb: { mode: 'physical', query: true, defrag: true, - hint: 'YugabyteDB: tries pgstattuple-compatible probe; falls back to custom SQL on failure.', + hint: 'YugabyteDB: tries the pgstatindex probe; falls back to custom SQL on failure.', customSqlHint: CUSTOM_HINT, }, mysql: { @@ -243,7 +247,9 @@ ORDER BY i.name sql: ` SELECT ci.relname AS index_name, - (SELECT leaf_fragmentation FROM pgstattuple(ci.oid)) AS fragmentation_percent, + -- pgstatindex reports NaN for an index with no leaf pages yet; that is + -- "nothing measured", not a number, and "NaN%" in the grid reads as a bug. + NULLIF((SELECT leaf_fragmentation FROM pgstatindex(ci.oid::regclass)), 'NaN'::float8) AS fragmentation_percent, NULL::bigint AS page_count FROM pg_index ix JOIN pg_class ct ON ct.oid = ix.indrelid @@ -585,7 +591,7 @@ WHERE i.name IS NOT NULL AND ps.index_level = 0;`; if (dialect === 'postgres' || dialect === 'cockroachdb' || dialect === 'yugabytedb') { return `-- Requires: CREATE EXTENSION IF NOT EXISTS pgstattuple; SELECT ci.relname AS index_name, - (pgstattuple(ci.oid)).leaf_fragmentation AS fragmentation_percent + (pgstatindex(ci.oid::regclass)).leaf_fragmentation AS fragmentation_percent FROM pg_index ix JOIN pg_class ct ON ct.oid = ix.indrelid JOIN pg_namespace n ON n.oid = ct.relnamespace