From 0b7c86fb694f6c34b6173d17511498ea3ce2c0b2 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sun, 20 Sep 2026 16:23:34 +0300 Subject: [PATCH] fix(postgres): ORDER BY PK on Table Browser and skip COUNT(*) Browse stays read-only. Row totals use reltuples so a sequential COUNT cannot forceClose the pooled socket. Stale estimates below the page leave Next enabled. --- CHANGELOG.md | 1 + lib/core/database/postgres_connection.dart | 31 +++++++++++ .../database/postgres_connection_pool.dart | 2 +- .../postgresql/postgres_table_utils.dart | 14 +++++ .../postgresql/postgres_table_view.dart | 52 ++++++++++--------- ...postgres_connection_string_parse_test.dart | 10 ++++ .../postgresql/postgres_table_utils_test.dart | 38 ++++++++++++++ 7 files changed, 122 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2fdb4dc..d17a2ecc 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 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. - **Postgres custom-SQL allowlist (#791)** — Table Browser SQL dialog classifies the first statement after comments instead of substring `contains('insert ')`. `SELECT inserted_at` is allowed; `SELECT 1; DELETE FROM t` is rejected. `WITH … INSERT` is a write; `TABLE` / `VALUES` / `(SELECT …)` stay allowed. diff --git a/lib/core/database/postgres_connection.dart b/lib/core/database/postgres_connection.dart index e6190e34..09670961 100644 --- a/lib/core/database/postgres_connection.dart +++ b/lib/core/database/postgres_connection.dart @@ -54,6 +54,14 @@ SslMode postgresResolveSslMode({ return SslMode.require; } +/// `pg_class.reltuples` → row estimate. Negative / unanalyzed → `null`. +int? postgresReltuplesEstimate(Object? value) { + if (value == null) return null; + final n = value is num ? value.toDouble() : double.tryParse(value.toString()); + if (n == null || n < 0) return null; + return n.round(); +} + /// PostgreSQL connection using the pure-Dart `postgres` package. class PostgresConnection { PostgresConnection({ @@ -490,6 +498,29 @@ class PostgresConnection { ); } + /// Planner row estimate (`pg_class.reltuples`), not a blocking `COUNT(*)`. + /// Unanalyzed relations (`-1`) and missing catalog rows return `null`. + Future estimateTableRows({ + String schema = 'public', + required String table, + }) async { + if (!isConnected || _conn == null) { + throw StateError('Not connected to PostgreSQL'); + } + final result = await _conn!.execute( + Sql.named( + 'SELECT c.reltuples FROM pg_catalog.pg_class c ' + 'JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace ' + 'WHERE n.nspname = @schema AND c.relname = @table ' + "AND c.relkind IN ('r', 'p', 'm', 'f') " + 'LIMIT 1', + ), + parameters: {'schema': schema, 'table': table}, + ); + if (result.isEmpty) return null; + return postgresReltuplesEstimate(result.first[0]); + } + /// Returns primary key column names for [table] in [schema]. Future> getPrimaryKeys({ String schema = 'public', diff --git a/lib/core/database/postgres_connection_pool.dart b/lib/core/database/postgres_connection_pool.dart index df5469f5..575fbf53 100644 --- a/lib/core/database/postgres_connection_pool.dart +++ b/lib/core/database/postgres_connection_pool.dart @@ -7,7 +7,7 @@ import 'package:querya_desktop/core/storage/local_db.dart'; /// Session policy for pooled connections: browse-only vs ad-hoc SQL (writes). enum PgSessionMode { /// `SET default_transaction_read_only = ON` after connect. - /// Tree catalog, stats, and Table Browser SELECT/COUNT. + /// Tree catalog, stats, and Table Browser SELECT. readOnly, /// SQL editor read-write session. Must not be shared with Table Browser. diff --git a/lib/features/postgresql/postgres_table_utils.dart b/lib/features/postgresql/postgres_table_utils.dart index 029c635c..a91d252e 100644 --- a/lib/features/postgresql/postgres_table_utils.dart +++ b/lib/features/postgresql/postgres_table_utils.dart @@ -7,6 +7,20 @@ import 'package:querya_desktop/core/database/sql_limit.dart'; /// Default page size for table browse and the SQL template filled from the tree. const kPostgresBrowseDefaultRowLimit = 200; +/// Browse SELECT for Table Browser. PK columns get `ORDER BY` so LIMIT/OFFSET +/// is stable. Empty [primaryKeys] keeps unordered scan (views / no PK). +String postgresBrowseDataSql({ + required String qualifiedFrom, + required List primaryKeys, + required int limit, + required int offset, +}) { + final order = primaryKeys.isEmpty + ? '' + : ' ORDER BY ${primaryKeys.map(quotePostgresIdentifier).join(', ')}'; + return 'SELECT * FROM $qualifiedFrom$order LIMIT $limit OFFSET $offset'; +} + /// `SELECT *` template matching [PostgresTableView] browse (same limit/offset). String postgresBrowseSelectSql({ required String schema, diff --git a/lib/features/postgresql/postgres_table_view.dart b/lib/features/postgresql/postgres_table_view.dart index e06fdbbd..eed61a36 100644 --- a/lib/features/postgresql/postgres_table_view.dart +++ b/lib/features/postgresql/postgres_table_view.dart @@ -59,7 +59,7 @@ class _PostgresTableViewState extends material.State { /// Rows on the current page (same as _rows.length when not loading). int _rowsOnPage = 0; - /// Total rows in table/view (from COUNT(*)). + /// Planner estimate (`reltuples`); `null` when unknown or stale. int? _totalRowCount; /// Zero-based offset for LIMIT/OFFSET pagination. @@ -182,18 +182,15 @@ class _PostgresTableViewState extends material.State { } } - static int _asInt(dynamic v) { - if (v == null) return 0; - if (v is int) return v; - if (v is BigInt) return v.toInt(); - if (v is num) return v.toInt(); - return int.tryParse(v.toString()) ?? 0; - } - String _browseDataSql() { final schemaQ = quotePostgresIdentifier(widget.schema); final tableQ = quotePostgresIdentifier(widget.tableName); - return 'SELECT * FROM $schemaQ.$tableQ LIMIT ${widget.limit} OFFSET $_offset'; + return postgresBrowseDataSql( + qualifiedFrom: '$schemaQ.$tableQ', + primaryKeys: _primaryKeys, + limit: widget.limit, + offset: _offset, + ); } Future _withTableWrite( @@ -275,7 +272,7 @@ class _PostgresTableViewState extends material.State { return convertResultRowsToStringsAdaptive(converted); } - /// [refreshCount] runs `COUNT(*)` (e.g. first load or Refresh). Pagination only runs SELECT. + /// [refreshCount] re-reads `reltuples` (e.g. first load or Refresh). Pagination only runs SELECT. Future _fetch({bool refreshCount = false}) async { final conn = _connection; if (conn == null || !conn.isConnected) { @@ -296,25 +293,26 @@ class _PostgresTableViewState extends material.State { _loading = true; _error = null; }); - final schemaQ = quotePostgresIdentifier(widget.schema); - final tableQ = quotePostgresIdentifier(widget.tableName); - final countSql = 'SELECT COUNT(*) AS c FROM $schemaQ.$tableQ'; - final dataSql = _browseDataSql(); try { - int totalRows; - if (refreshCount || _totalRowCount == null) { - final countResult = await conn.execute(countSql); - totalRows = countResult.isEmpty ? 0 : _asInt(countResult.first[0]); - } else { - totalRows = _totalRowCount!; + await _ensureSchema(conn); + if (!mounted) return; + + int? totalRows = _totalRowCount; + if (refreshCount || totalRows == null) { + try { + totalRows = await conn.estimateTableRows( + schema: widget.schema, + table: widget.tableName, + ); + } catch (_) { + totalRows = null; + } } + final dataSql = _browseDataSql(); final result = await conn.execute(dataSql); if (!mounted) return; - await _ensureSchema(conn); - if (!mounted) return; - final colNames = List.generate( result.schema.columns.length, (i) => result.schema.columns[i].columnName ?? 'col_$i', @@ -323,10 +321,14 @@ class _PostgresTableViewState extends material.State { final stringRows = await _postgresRowsToDisplayStrings(result, colNames); if (!mounted) return; + final shown = stringRows.length; + if (totalRows != null && shown > 0 && totalRows < _offset + shown) { + totalRows = null; + } setState(() { _columnNames = colNames; _rows = stringRows; - _rowsOnPage = stringRows.length; + _rowsOnPage = shown; if (refreshCount || _totalRowCount == null) { _totalRowCount = totalRows; } diff --git a/test/core/database/postgres_connection_string_parse_test.dart b/test/core/database/postgres_connection_string_parse_test.dart index 8d120fc3..5f1f95f5 100644 --- a/test/core/database/postgres_connection_string_parse_test.dart +++ b/test/core/database/postgres_connection_string_parse_test.dart @@ -109,4 +109,14 @@ void main() { ); }); }); + + group('postgresReltuplesEstimate', () { + test('rounds planner floats and treats unanalyzed as unknown', () { + expect(postgresReltuplesEstimate(200.4), 200); + expect(postgresReltuplesEstimate(199.6), 200); + expect(postgresReltuplesEstimate(0), 0); + expect(postgresReltuplesEstimate(-1), isNull); + expect(postgresReltuplesEstimate(null), isNull); + }); + }); } diff --git a/test/features/postgresql/postgres_table_utils_test.dart b/test/features/postgresql/postgres_table_utils_test.dart index 1db0508d..db70c9b9 100644 --- a/test/features/postgresql/postgres_table_utils_test.dart +++ b/test/features/postgresql/postgres_table_utils_test.dart @@ -90,6 +90,44 @@ void main() { }); }); + group('postgresBrowseDataSql', () { + test('orders by quoted PK columns', () { + expect( + postgresBrowseDataSql( + qualifiedFrom: '"public"."orders"', + primaryKeys: const ['id'], + limit: 200, + offset: 400, + ), + 'SELECT * FROM "public"."orders" ORDER BY "id" LIMIT 200 OFFSET 400', + ); + }); + + test('composite PK lists all columns', () { + expect( + postgresBrowseDataSql( + qualifiedFrom: '"public"."t"', + primaryKeys: const ['a', 'b'], + limit: 200, + offset: 0, + ), + 'SELECT * FROM "public"."t" ORDER BY "a", "b" LIMIT 200 OFFSET 0', + ); + }); + + test('omits ORDER BY when there is no PK', () { + expect( + postgresBrowseDataSql( + qualifiedFrom: '"public"."v"', + primaryKeys: const [], + limit: 200, + offset: 0, + ), + 'SELECT * FROM "public"."v" LIMIT 200 OFFSET 0', + ); + }); + }); + group('convertResultRowsToStrings', () { test('converts null to "NULL"', () { final result = convertResultRowsToStrings([