diff --git a/CHANGELOG.md b/CHANGELOG.md index d17a2ec..7ee59ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Postgres open-transaction probe (#787)** — `BEGIN` + `SELECT` does not assign an XID, so `pg_current_xact_id_if_assigned` stayed NULL (SQL-tab badge off; autocommit-off prepended a second `BEGIN`). The session tracks BEGIN/COMMIT/ROLLBACK and otherwise probes `pg_stat_activity.xact_start` (PG 9+). Implicit BEGIN when autocommit is off stays a separate `execute`. - **Postgres Table Browser paging (#788)** — Browse stays on `PgSessionMode.readOnly`. `SELECT` uses `ORDER BY` primary-key columns when a PK exists. Row totals come from `pg_class.reltuples` instead of a blocking `COUNT(*)` before first paint (stale estimates below the current page are ignored so Next still works). Save / REFRESH stay on `tableWrite`. - **Postgres timestamptz / bytea / jsonb / arrays (#789)** — Table Browser and SQL results encode cells as PG literals (ISO timestamps, `\x` hex for bytea, JSON text for jsonb, `{1,2,3}` for arrays) instead of `Object.toString`. Schema uses `udt_name` so `ARRAY` / `USER-DEFINED` round-trip through `formatLiteral`. - **Postgres host/port SSL (#790)** — Host/port Use SSL/TLS stays `sslmode=require` (encrypt, no CA check) unless a Root CA is set, then `verify-full`. A URI `sslmode=` value still wins. The form documents the MITM tradeoff and writes `sslmode=verify-full` when saving a Root CA. diff --git a/lib/core/database/postgres_connection.dart b/lib/core/database/postgres_connection.dart index 0967096..8507de0 100644 --- a/lib/core/database/postgres_connection.dart +++ b/lib/core/database/postgres_connection.dart @@ -11,6 +11,7 @@ import 'package:postgres/src/connection_string.dart' show parseConnectionString; import 'postgres_metadata.dart'; import 'postgres_result_cells.dart'; +import 'postgres_sql.dart'; import 'table_schema_meta.dart'; /// Replaces the database in a `postgresql://` / `postgres://` URI (path or @@ -131,6 +132,7 @@ class PostgresConnection { Connection? _conn; bool _isConnected = false; + bool _inTransaction = false; bool get isConnected => _isConnected && _conn != null; @@ -238,6 +240,7 @@ class PostgresConnection { ); } _isConnected = true; + _inTransaction = false; scrubCredentials(); } catch (e, st) { _isConnected = false; @@ -255,6 +258,7 @@ class PostgresConnection { Future disconnect() async { _isConnected = false; + _inTransaction = false; final c = _conn; _conn = null; try { @@ -268,6 +272,7 @@ class PostgresConnection { /// cancelling a long query or [PostgresService.interrupt]. Future forceClose() async { _isConnected = false; + _inTransaction = false; final c = _conn; _conn = null; try { @@ -313,7 +318,9 @@ class PostgresConnection { throw StateError('Not connected to PostgreSQL'); } try { - return await _conn!.execute(sql, timeout: timeout); + final result = await _conn!.execute(sql, timeout: timeout); + _inTransaction = applyPostgresTransactionSql(_inTransaction, sql); + return result; } on TimeoutException { unawaited(forceClose()); rethrow; @@ -335,19 +342,24 @@ class PostgresConnection { } } - /// Whether the session has an open transaction (PostgreSQL 13+). - /// Returns `null` if the server does not support the probe or an error occurs. + /// Whether the session has an open transaction. + /// + /// `BEGIN` + `SELECT` does not assign an XID, so `pg_current_xact_id_if_assigned` + /// stays NULL. We track BEGIN/COMMIT/ROLLBACK on [execute], and otherwise + /// probe `pg_stat_activity.xact_start` for this backend (PG 9+; no PG 13 + /// requirement). Returns `null` only when disconnected. Future inOpenTransaction() async { if (!isConnected || _conn == null) return null; + if (_inTransaction) return true; try { - final r = await _conn!.execute( - 'SELECT pg_current_xact_id_if_assigned() IS NOT NULL', - ); - if (r.isEmpty) return null; - return r.first[0] as bool; + final r = await _conn!.execute(kPostgresOpenTransactionProbeSql); + if (r.isEmpty) return false; + final open = r.first[0] == true; + _inTransaction = open; + return open; } catch (e) { debugPrint('PostgresConnection.inOpenTransaction: $e'); - return null; + return _inTransaction; } } diff --git a/lib/core/database/postgres_sql.dart b/lib/core/database/postgres_sql.dart index d565506..fd746b0 100644 --- a/lib/core/database/postgres_sql.dart +++ b/lib/core/database/postgres_sql.dart @@ -27,6 +27,47 @@ bool shouldSkipImplicitBegin(String sql) { return false; } +/// Own-backend `xact_start` (set at `BEGIN`, even when no XID is assigned). +/// +/// Replaces `pg_current_xact_id_if_assigned()` (PG 13+), which stays NULL after +/// `BEGIN` + `SELECT`. `pg_stat_activity.xact_start` exists on PG 9+. +const kPostgresOpenTransactionProbeSql = + 'SELECT xact_start IS NOT NULL ' + 'FROM pg_catalog.pg_stat_activity ' + 'WHERE pid = pg_backend_pid()'; + +/// Updates session tx state from a successfully executed statement. +/// +/// `BEGIN` / `START TRANSACTION` open; `COMMIT` / `END` / `ROLLBACK` close. +/// `ROLLBACK TO` (savepoint) leaves the transaction open. +bool applyPostgresTransactionSql(bool currentlyOpen, String sql) { + final s = stripLeadingWhitespaceAndLineComments(sql); + if (s.isEmpty) return currentlyOpen; + final u = s.toUpperCase(); + + if (u.startsWith('START TRANSACTION') || _pgTxStartsWithKeyword(u, 'BEGIN')) { + return true; + } + if (_pgTxStartsWithKeyword(u, 'COMMIT') || _pgTxStartsWithKeyword(u, 'END')) { + return false; + } + if (_pgTxStartsWithKeyword(u, 'ROLLBACK')) { + if (RegExp(r'^ROLLBACK\s+TO\b').hasMatch(u)) return currentlyOpen; + return false; + } + return currentlyOpen; +} + +bool _pgTxStartsWithKeyword(String upper, String kw) { + if (!upper.startsWith(kw)) return false; + if (upper.length == kw.length) return true; + final next = upper.codeUnitAt(kw.length); + final isIdent = (next >= 0x41 && next <= 0x5a) || + next == 0x5f || + (next >= 0x30 && next <= 0x39); + return !isIdent; +} + /// Runs [statements] as separate extended-protocol executes inside BEGIN/COMMIT. /// /// PostgreSQL Parse cannot contain multiple commands, so diff --git a/lib/features/postgresql/postgres_sql_workspace.dart b/lib/features/postgresql/postgres_sql_workspace.dart index 8574f96..b6a06fd 100644 --- a/lib/features/postgresql/postgres_sql_workspace.dart +++ b/lib/features/postgresql/postgres_sql_workspace.dart @@ -450,6 +450,7 @@ class _PostgresSqlWorkspaceState extends material.State { if (!_autocommit) { final inTx = await conn.inOpenTransaction() ?? false; if (!inTx && !shouldSkipImplicitBegin(sql)) { + // Separate execute: Parse cannot take `BEGIN;` + the next statement. await conn.execute('BEGIN', timeout: to); } } diff --git a/test/core/database/postgres_sql_test.dart b/test/core/database/postgres_sql_test.dart index fd69b46..efec076 100644 --- a/test/core/database/postgres_sql_test.dart +++ b/test/core/database/postgres_sql_test.dart @@ -80,6 +80,60 @@ void main() { }); }); + group('applyPostgresTransactionSql', () { + test('BEGIN + SELECT stays open; COMMIT closes', () { + var open = false; + open = applyPostgresTransactionSql(open, 'BEGIN'); + expect(open, isTrue); + open = applyPostgresTransactionSql(open, 'SELECT 1'); + expect(open, isTrue); + open = applyPostgresTransactionSql(open, 'COMMIT'); + expect(open, isFalse); + }); + + test('BEGIN WORK opens; BEGINNING does not', () { + expect(applyPostgresTransactionSql(false, 'BEGIN WORK'), isTrue); + expect(applyPostgresTransactionSql(false, 'BEGINNING'), isFalse); + }); + + test('START TRANSACTION and END', () { + var open = applyPostgresTransactionSql(false, 'START TRANSACTION'); + expect(open, isTrue); + open = applyPostgresTransactionSql(open, 'END'); + expect(open, isFalse); + }); + + test('ROLLBACK closes; ROLLBACK TO savepoint does not', () { + expect(applyPostgresTransactionSql(true, 'ROLLBACK'), isFalse); + expect( + applyPostgresTransactionSql(true, 'ROLLBACK TO sp1'), + isTrue, + ); + expect( + applyPostgresTransactionSql(true, 'ROLLBACK TO SAVEPOINT sp1'), + isTrue, + ); + }); + + test('strips leading comments before BEGIN', () { + expect( + applyPostgresTransactionSql(false, '-- note\nBEGIN'), + isTrue, + ); + }); + }); + + group('kPostgresOpenTransactionProbeSql', () { + test('uses xact_start, not xid-if-assigned', () { + expect(kPostgresOpenTransactionProbeSql, contains('xact_start')); + expect(kPostgresOpenTransactionProbeSql, contains('pg_backend_pid()')); + expect( + kPostgresOpenTransactionProbeSql, + isNot(contains('pg_current_xact_id')), + ); + }); + }); + group('runPostgresStatementsInTransaction', () { test('executes BEGIN, each statement, COMMIT as separate calls', () async { final calls = [];