From fe51ab235310c896bde62c787306c4d3258707e1 Mon Sep 17 00:00:00 2001 From: huyplb Date: Tue, 18 Aug 2026 17:51:45 -0600 Subject: [PATCH 1/2] fix(sync): selecting a saved SQLite connection asked for a password it cannot have MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A real break, found by running the revert e2e before releasing rather than after. `selectSavedConnection` treats "no stored password" as "prompt the user", which was never reached for SQLite while the credential form still offered a Save-password box. #271 hid that box for file dialects — correctly, a file has no password — so SQLite credentials now save with `hasPassword: false`, and picking one as Source or Target opened a password prompt, snapped the picker back to "— Saved —", and left no connection selected. Snapshot, Compare and Migrate all stayed disabled with nothing on screen explaining why. Reproduced by hand in the browser, not inferred from the test: pick a saved SQLite credential, watch the select revert. `isFileDialect` / `dialectUsesPassword` now live in `provider-settings` beside the rest of the per-dialect truth, and both the picker and the connection modal read them instead of each carrying their own copy of the list. Also updates the last e2e that asserted the old filter behaviour: it checked `isChecked()` on Function and Procedure boxes, which this seed's schema does not have and which are therefore no longer rendered — an assertion on an absent control hangs rather than fails. E2E after this, against the running app: schema-revert 4/4, schema-version-revert-edges 10/10, schema-history 6/6. Co-Authored-By: Claude Opus 5 --- apps/e2e/src/tests/schema-history.test.ts | 8 +++++--- .../frontend/components/ConnectionModal.tsx | 12 ++++++++++-- .../web/src/frontend/components/TopToolbar.tsx | 5 ++++- apps/web/src/frontend/lib/provider-settings.ts | 18 ++++++++++++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) 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/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]) ); From 4c2bf253e6e5a51c20ba7982273df9239188c6a5 Mon Sep 17 00:00:00 2001 From: huyplb Date: Tue, 18 Aug 2026 18:02:14 -0600 Subject: [PATCH 2/2] fix(utilities): index fragmentation named the wrong Postgres function, and MariaDB is not MySQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reported failures, both reproduced against the live engines rather than reasoned about. **Index fragmentation on PostgreSQL could never have worked.** The probe read `SELECT leaf_fragmentation FROM pgstattuple(ci.oid)`. Against PostgreSQL 17: -- extension absent (the default, and the reported error) ERROR: function pgstattuple(oid) does not exist -- extension installed — the obvious fix ERROR: column "leaf_fragmentation" does not exist `pgstattuple` reports *table* statistics. `leaf_fragmentation` belongs to `pgstatindex`, which returns 0 for a real index. So installing the extension swapped one error for another; the feature was broken either way. Now uses `pgstatindex(ci.oid::regclass)`, verified returning values on that server. Two follow-ons. `pgstatindex` yields NaN for an index with no leaf pages yet — "nothing measured", not a number, and `NaN%` in a column reads as a bug — so it is nulled out. And a missing extension now explains itself: the panel says the server needs `CREATE EXTENSION pgstattuple;` and that a superuser can add it, instead of relaying the driver's sentence. **MariaDB was aliased to the MySQL family and two probes were wrong on it.** Against MariaDB 11.8: SELECT @@innodb_buffer_pool_instances; -- ERROR 1193: Unknown system variable SELECT ... FROM performance_schema.global_status ... -- (empty) SELECT ... FROM information_schema.GLOBAL_STATUS ... -- 245412 The removed variable killed System info outright, which is the reported bug. The empty one was quieter and worse: performance_schema is off by default on MariaDB, so the pool panel showed blank connection counts with no error at all. MariaDB is now its own family, reading status from `information_schema` and dropping the variable it no longer has; sessions and sizes share the MySQL queries, which do work on it. Verified live, all four utilities: pool 151 max / 1 connected, sessions listed, system up 245536s on 11.8.8-MariaDB, sizes reported per table. Co-Authored-By: Claude Opus 5 --- .../src/backend/api/index-fragmentation.ts | 23 ++++++++- .../src/modules/dialect-dba-utilities.test.ts | 51 +++++++++++++++++++ .../sql/src/modules/dialect-dba-utilities.ts | 50 ++++++++++++++++-- .../dialect-index-fragmentation.test.ts | 27 +++++++++- .../modules/dialect-index-fragmentation.ts | 18 ++++--- 5 files changed, 157 insertions(+), 12 deletions(-) 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/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