From 751c9a904b5ac3e769061ce00b2fedb28b6bfa70 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 2 Nov 2026 17:12:06 +0100 Subject: [PATCH 1/2] fix: count returned rows in history for statements with a result set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit History and the session log summed `affectedRows ?? rowCount`. Drivers report `affectedRows: 0` for a SELECT — MySQL always did, and since Bun 1.4 Bun.SQL does the same for PostgreSQL and SQLite — so a read was logged as touching 0 rows. CI picked up Bun 1.4.4 and the history test started failing on main. `statementRowCount` counts what a statement returned when it produced a result set, and what it changed otherwise. The backend history, session log, demo adapter and the frontend history entry all use it. Co-Authored-By: Claude Opus 5.5 --- src/backend-shared/rpc/backend-adapter.ts | 3 ++- src/backend-shared/services/query-executor.ts | 3 ++- src/frontend-demo/demo-adapter.ts | 3 ++- src/frontend-shared/stores/editor.ts | 3 ++- src/shared/query-result.ts | 9 +++++++++ tests/rpc-handlers.test.ts | 12 ++++++++++++ 6 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 src/shared/query-result.ts diff --git a/src/backend-shared/rpc/backend-adapter.ts b/src/backend-shared/rpc/backend-adapter.ts index b348d4a..36cbb36 100644 --- a/src/backend-shared/rpc/backend-adapter.ts +++ b/src/backend-shared/rpc/backend-adapter.ts @@ -1,3 +1,4 @@ +import { statementRowCount } from '@dotaz/shared/query-result' import type { ConnectionConfig, ConnectionInfo, ConnectionSecrets } from '@dotaz/shared/types/connection' import { mergeSecrets } from '@dotaz/shared/types/connection' import type { DatabaseInfo } from '@dotaz/shared/types/database' @@ -282,7 +283,7 @@ export class BackendAdapter implements RpcAdapter { stmt.sql, result.error ? 'error' : 'success', durationMs, - result.affectedRows ?? result.rowCount, + statementRowCount(result), result.error, database, effectiveSessionId, diff --git a/src/backend-shared/services/query-executor.ts b/src/backend-shared/services/query-executor.ts index 2858f15..c077016 100644 --- a/src/backend-shared/services/query-executor.ts +++ b/src/backend-shared/services/query-executor.ts @@ -1,3 +1,4 @@ +import { statementRowCount } from '@dotaz/shared/query-result' import { parseErrorPosition, splitStatements, stripLiteralsAndComments } from '@dotaz/shared/sql/statements' import { DatabaseError } from '@dotaz/shared/types/errors' import type { ExplainNode, ExplainResult, QueryResult } from '@dotaz/shared/types/query' @@ -516,7 +517,7 @@ export class QueryExecutor { private logHistory(connectionId: string, sql: string, results: QueryResult[], database?: string, sessionId?: string): void { const hasError = results.some((r) => r.error) const totalDuration = results.reduce((sum, r) => sum + (r.durationMs ?? 0), 0) - const totalRows = results.reduce((sum, r) => sum + (r.affectedRows ?? r.rowCount), 0) + const totalRows = results.reduce((sum, r) => sum + statementRowCount(r), 0) const errorMessage = results.find((r) => r.error)?.error // Always add to session log (in-memory) diff --git a/src/frontend-demo/demo-adapter.ts b/src/frontend-demo/demo-adapter.ts index d3856aa..964c2ad 100644 --- a/src/frontend-demo/demo-adapter.ts +++ b/src/frontend-demo/demo-adapter.ts @@ -6,6 +6,7 @@ import type { ExportWriter } from '@dotaz/backend-shared/services/export-service import { importFromStream, importPreviewFromStream } from '@dotaz/backend-shared/services/import-service' import { searchDatabase } from '@dotaz/backend-shared/services/search-service' import { formatSql } from '@dotaz/backend-shared/services/sql-formatter' +import { statementRowCount } from '@dotaz/shared/query-result' import { splitStatements } from '@dotaz/shared/sql/statements' import { type ConnectionConfig, type ConnectionInfo, getDefaultDatabase } from '@dotaz/shared/types/connection' import type { DatabaseInfo } from '@dotaz/shared/types/database' @@ -633,7 +634,7 @@ export class DemoAdapter implements RpcAdapter { private logHistory(connectionId: string, sql: string, results: QueryResult[]): void { const hasError = results.some((r) => r.error) const totalDuration = results.reduce((sum, r) => sum + (r.durationMs ?? 0), 0) - const totalRows = results.reduce((sum, r) => sum + (r.affectedRows ?? r.rowCount), 0) + const totalRows = results.reduce((sum, r) => sum + statementRowCount(r), 0) const errorMessage = results.find((r) => r.error)?.error // Add to session log (in-memory) diff --git a/src/frontend-shared/stores/editor.ts b/src/frontend-shared/stores/editor.ts index d3abc5b..259c1fb 100644 --- a/src/frontend-shared/stores/editor.ts +++ b/src/frontend-shared/stores/editor.ts @@ -1,3 +1,4 @@ +import { statementRowCount } from '@dotaz/shared/query-result' import { generateChangesPreview, generateChangeSql } from '@dotaz/shared/sql/builders' import { detectDestructiveWithoutWhere, isUnlimitedSelect, splitStatements } from '@dotaz/shared/sql/statements' import type { ExplainResult, QueryEditability, QueryResult } from '@dotaz/shared/types/query' @@ -270,7 +271,7 @@ function setCursorPosition(tabId: string, position: number) { function recordHistory(connectionId: string, database: string | undefined, sql: string, results: QueryResult[]) { const hasError = results.some((r) => r.error) const totalDuration = results.reduce((sum, r) => sum + (r.durationMs ?? 0), 0) - const totalRows = results.reduce((sum, r) => sum + (r.affectedRows ?? r.rowCount ?? 0), 0) + const totalRows = results.reduce((sum, r) => sum + statementRowCount(r), 0) storage.addHistoryEntry({ connectionId, database, diff --git a/src/shared/query-result.ts b/src/shared/query-result.ts new file mode 100644 index 0000000..55cf049 --- /dev/null +++ b/src/shared/query-result.ts @@ -0,0 +1,9 @@ +import type { QueryResult } from './types/query' + +/** + * Rows one statement accounts for: what it returned when it produced a result set, what it + * changed otherwise. `affectedRows` alone is not enough — drivers report 0 for a SELECT. + */ +export function statementRowCount(result: QueryResult): number { + return result.columns.length > 0 ? result.rowCount : (result.affectedRows ?? result.rowCount) +} diff --git a/tests/rpc-handlers.test.ts b/tests/rpc-handlers.test.ts index e9b7fe8..8ebb6b7 100644 --- a/tests/rpc-handlers.test.ts +++ b/tests/rpc-handlers.test.ts @@ -524,6 +524,18 @@ describe('RPC Handlers', () => { expect(history[0].rowCount).toBe(1) }) + // Drivers report `affectedRows: 0` for a SELECT (MySQL always, PostgreSQL/SQLite since Bun 1.4), + // so history must count what a result set returned, and what a write changed. + test('history counts returned rows for a read and changed rows for a write', async () => { + await handlers['query.execute']({ connectionId, sql: 'CREATE TABLE hist_t (a INTEGER)', queryId: 'h-c' }) + await handlers['query.execute']({ connectionId, sql: 'INSERT INTO hist_t VALUES (1), (2), (3)', queryId: 'h-i' }) + await handlers['query.execute']({ connectionId, sql: 'SELECT a FROM hist_t WHERE a > 1', queryId: 'h-s' }) + + const bySql = new Map(handlers['history.list']({}).map((entry) => [entry.sql, entry.rowCount])) + expect(bySql.get('INSERT INTO hist_t VALUES (1), (2), (3)')).toBe(3) + expect(bySql.get('SELECT a FROM hist_t WHERE a > 1')).toBe(2) + }) + test('failed queries are logged with error status', async () => { await handlers['query.execute']({ connectionId, From 9f9dc75d698abb3f27f939a499ec71fe3f6dc7a8 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 2 Nov 2026 17:15:23 +0100 Subject: [PATCH 2/2] test: pin the zero-row read case of statementRowCount Also say what the columns check actually means and list query-result.ts in the shared module map. Co-Authored-By: Claude Opus 5.5 --- src/shared/CLAUDE.md | 1 + src/shared/query-result.ts | 5 +++-- tests/rpc-handlers.test.ts | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/shared/CLAUDE.md b/src/shared/CLAUDE.md index 4ca3c6d..cf524ed 100644 --- a/src/shared/CLAUDE.md +++ b/src/shared/CLAUDE.md @@ -12,6 +12,7 @@ src/shared/ clipboard-paste.ts ← Clipboard paste parsing column-types.ts ← Column type utilities comparison.ts ← Data comparison logic + query-result.ts ← Row count a statement accounts for (history, session log) ``` ## Types (`src/shared/types/`) diff --git a/src/shared/query-result.ts b/src/shared/query-result.ts index 55cf049..6e2ce69 100644 --- a/src/shared/query-result.ts +++ b/src/shared/query-result.ts @@ -1,8 +1,9 @@ import type { QueryResult } from './types/query' /** - * Rows one statement accounts for: what it returned when it produced a result set, what it - * changed otherwise. `affectedRows` alone is not enough — drivers report 0 for a SELECT. + * Rows one statement accounts for: what it returned when it returned rows, what it changed + * otherwise. `affectedRows` alone is not enough — drivers report 0 for a SELECT. Drivers derive + * `columns` from the first row, so a zero-row read takes the `affectedRows` branch, which is 0. */ export function statementRowCount(result: QueryResult): number { return result.columns.length > 0 ? result.rowCount : (result.affectedRows ?? result.rowCount) diff --git a/tests/rpc-handlers.test.ts b/tests/rpc-handlers.test.ts index 8ebb6b7..231ba45 100644 --- a/tests/rpc-handlers.test.ts +++ b/tests/rpc-handlers.test.ts @@ -530,10 +530,12 @@ describe('RPC Handlers', () => { await handlers['query.execute']({ connectionId, sql: 'CREATE TABLE hist_t (a INTEGER)', queryId: 'h-c' }) await handlers['query.execute']({ connectionId, sql: 'INSERT INTO hist_t VALUES (1), (2), (3)', queryId: 'h-i' }) await handlers['query.execute']({ connectionId, sql: 'SELECT a FROM hist_t WHERE a > 1', queryId: 'h-s' }) + await handlers['query.execute']({ connectionId, sql: 'SELECT a FROM hist_t WHERE a > 9', queryId: 'h-e' }) const bySql = new Map(handlers['history.list']({}).map((entry) => [entry.sql, entry.rowCount])) expect(bySql.get('INSERT INTO hist_t VALUES (1), (2), (3)')).toBe(3) expect(bySql.get('SELECT a FROM hist_t WHERE a > 1')).toBe(2) + expect(bySql.get('SELECT a FROM hist_t WHERE a > 9')).toBe(0) }) test('failed queries are logged with error status', async () => {