From 29acdf15196453b5d67dd787cc12e9ebbb452ac3 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Fri, 25 Sep 2026 14:48:18 +0300 Subject: [PATCH] perf(postgresql): convert result cells in one typed pass instead of two (Closes #874) The Table Browser and SQL workspace converted cells with the PG type OIDs and then pushed the resulting strings through convertResultRowsToStringsAdaptive, which walked the whole matrix again (and copied it to an isolate from 1000 rows) without being able to recover any type information. The typed pass also ran as one uninterrupted block on the main isolate. Add convertPostgresResultRowsToStringsAdaptive: one pass that converts with the PG type, interns the string, and yields to the event loop every 250 rows. Both callers use it and no longer run the adaptive string converter on PG rows. --- .../postgresql/postgres_result_utils.dart | 40 +++++++++ .../postgresql/postgres_sql_workspace.dart | 4 +- .../postgresql/postgres_table_view.dart | 4 +- .../result_conversion_perf_test.dart | 84 +++++++++++++++++++ 4 files changed, 126 insertions(+), 6 deletions(-) diff --git a/lib/features/postgresql/postgres_result_utils.dart b/lib/features/postgresql/postgres_result_utils.dart index da0e494..ef92651 100644 --- a/lib/features/postgresql/postgres_result_utils.dart +++ b/lib/features/postgresql/postgres_result_utils.dart @@ -1,4 +1,5 @@ import 'package:querya_desktop/core/database/postgres_result_cells.dart'; +import 'package:querya_desktop/core/database/result_row_string_convert.dart'; /// Serializable row batch for [convertPostgresResultRowsToStrings]. class PostgresResultConvertJob { @@ -30,3 +31,42 @@ List> convertPostgresResultRowsToStrings( ], ]; } + +/// Typed conversion in a single pass: converts each cell with its PG type, +/// interns the resulting string (repeated enum / status values share one +/// instance) and yields to the event loop every [yieldEvery] rows. +/// +/// Rows are already strings after this, so callers must not run them through +/// [convertResultRowsToStringsAdaptive] again: that would walk (and, from 1000 +/// rows, copy to an isolate) the whole matrix a second time without being able +/// to recover any type information. +Future>> convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob job, { + int yieldEvery = kResultStringConvertYieldEvery, + StringInternPool? pool, +}) async { + final rowValues = job.rowValues; + if (rowValues.isEmpty) return const []; + + final oids = job.columnTypeOids; + final types = job.columnDataTypes; + final activePool = pool ?? StringInternPool(); + final out = >[]; + for (var r = 0; r < rowValues.length; r++) { + final row = rowValues[r]; + out.add([ + for (var i = 0; i < row.length; i++) + activePool.intern( + postgresResultCellToDisplayString( + row[i], + typeOid: oids != null && i < oids.length ? oids[i] : null, + dataTypeName: types != null && i < types.length ? types[i] : null, + ), + ), + ]); + if (yieldEvery > 0 && (r + 1) % yieldEvery == 0) { + await Future.delayed(Duration.zero); + } + } + return out; +} diff --git a/lib/features/postgresql/postgres_sql_workspace.dart b/lib/features/postgresql/postgres_sql_workspace.dart index 87fc520..f0019da 100644 --- a/lib/features/postgresql/postgres_sql_workspace.dart +++ b/lib/features/postgresql/postgres_sql_workspace.dart @@ -10,7 +10,6 @@ import 'package:postgres/postgres.dart' as pg; import 'package:querya_desktop/core/database/destructive_sql_detector.dart'; import 'package:querya_desktop/core/database/postgres_service.dart'; import 'package:querya_desktop/core/database/postgres_sql.dart'; -import 'package:querya_desktop/core/database/result_row_string_convert.dart'; import 'package:querya_desktop/core/database/sql_table_target_extractor.dart'; import 'package:querya_desktop/core/database/table_mutation_engine.dart'; import 'package:querya_desktop/core/layout/vertical_split_pane.dart'; @@ -504,7 +503,7 @@ class _PostgresSqlWorkspaceState extends material.State { n++; } - final converted = convertPostgresResultRowsToStrings( + final outRows = await convertPostgresResultRowsToStringsAdaptive( PostgresResultConvertJob( rowValues: rawRows, columnTypeOids: [ @@ -512,7 +511,6 @@ class _PostgresSqlWorkspaceState extends material.State { ], ), ); - final outRows = await convertResultRowsToStringsAdaptive(converted); final target = SqlTableTargetExtractor.extract(userSql); var pks = const []; diff --git a/lib/features/postgresql/postgres_table_view.dart b/lib/features/postgresql/postgres_table_view.dart index 1624423..bd2b6b5 100644 --- a/lib/features/postgresql/postgres_table_view.dart +++ b/lib/features/postgresql/postgres_table_view.dart @@ -6,7 +6,6 @@ import 'package:postgres/postgres.dart'; import 'package:querya_desktop/core/database/postgres_connection.dart'; import 'package:querya_desktop/core/database/postgres_service.dart'; import 'package:querya_desktop/core/database/postgres_sql.dart'; -import 'package:querya_desktop/core/database/result_row_string_convert.dart'; import 'package:querya_desktop/core/database/table_mutation_engine.dart'; import 'package:querya_desktop/core/database/table_schema_meta.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; @@ -268,7 +267,7 @@ class _PostgresTableViewState extends material.State { for (final row in result) List.generate(row.length, (i) => row[i]), ]; - final converted = convertPostgresResultRowsToStrings( + return convertPostgresResultRowsToStringsAdaptive( PostgresResultConvertJob( rowValues: rawRows, columnTypeOids: [ @@ -279,7 +278,6 @@ class _PostgresTableViewState extends material.State { ], ), ); - return convertResultRowsToStringsAdaptive(converted); } /// [refreshCount] re-reads `reltuples` (e.g. first load or Refresh). Pagination only runs SELECT. diff --git a/test/features/sql_workspaces/result_conversion_perf_test.dart b/test/features/sql_workspaces/result_conversion_perf_test.dart index b82c87a..9fe63a3 100644 --- a/test/features/sql_workspaces/result_conversion_perf_test.dart +++ b/test/features/sql_workspaces/result_conversion_perf_test.dart @@ -72,4 +72,88 @@ void main() { expect(sqliteOut[100][1], 'row_100_col_1'); }); }); + + group('convertPostgresResultRowsToStringsAdaptive (single typed pass)', () { + // Text OIDs plus a date OID: 1082 = date, 25 = text, 23 = int4. + const oids = [23, 25, 1082, 25]; + List> sample(int n) => [ + for (var i = 0; i < n; i++) + [i, i.isEven ? 'active' : 'custom_$i', DateTime.utc(2026, 3, 1 + i % 20), null], + ]; + + test('matches the typed conversion exactly', () async { + final job = PostgresResultConvertJob( + rowValues: sample(1200), + columnTypeOids: oids, + ); + expect( + await convertPostgresResultRowsToStringsAdaptive(job), + convertPostgresResultRowsToStrings(job), + ); + }); + + test('keeps PG typing: date-only OID renders as a date, NULL as NULL', + () async { + final out = await convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob(rowValues: sample(2), columnTypeOids: oids), + ); + expect(out[0], ['0', 'active', '2026-03-01', 'NULL']); + expect(out[1][2], '2026-03-02'); + }); + + test('uses column data type names when there is no OID', () async { + final out = await convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob( + rowValues: [ + [DateTime.utc(2026, 3, 1, 10)], + ], + columnDataTypes: const ['date'], + ), + ); + expect(out.single.single, '2026-03-01'); + }); + + test('interns repeated values so they share one string instance', () async { + final out = await convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob( + rowValues: [ + for (var i = 0; i < 5; i++) [i, 'status_${i % 2}_${'x' * 3}'], + ], + columnTypeOids: const [23, 25], + ), + ); + expect(identical(out[0][1], out[2][1]), isTrue); + expect(identical(out[1][1], out[3][1]), isTrue); + }); + + test('returns every row when yielding to the event loop', () async { + final out = await convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob(rowValues: sample(1000), columnTypeOids: oids), + yieldEvery: 7, + ); + expect(out, hasLength(1000)); + expect(out.last[0], '999'); + }); + + test('empty input yields empty output', () async { + expect( + await convertPostgresResultRowsToStringsAdaptive( + const PostgresResultConvertJob(rowValues: []), + ), + isEmpty, + ); + }); + + test('lets the event loop run while converting a large result', () async { + var ticks = 0; + final done = convertPostgresResultRowsToStringsAdaptive( + PostgresResultConvertJob(rowValues: sample(4000), columnTypeOids: oids), + yieldEvery: 100, + ); + // Each yield lets this microtask-scheduled timer fire before the end. + Future.delayed(Duration.zero, () => ticks++); + await done; + expect(ticks, 1, reason: 'a queued task ran while conversion was in flight'); + }); + }); }