From d27176aab705a72b3c4219d841c90a928b4caffb Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Mon, 27 Jul 2026 19:09:43 +0300 Subject: [PATCH] perf(sql): clamp oversized LIMIT / FETCH for result caps Extend injectSqlLimit to clamp LIMIT n, LIMIT ALL, and FETCH FIRST when larger than the UI row cap so Postgres (and SQLite) engines do not buffer more rows than Preferences allow. Document the policy. Closes #416 --- docs/user-guide.md | 7 ++- lib/core/database/sql_limit.dart | 57 +++++++++++++++++++---- test/core/database/postgres_sql_test.dart | 7 ++- test/core/database/sql_limit_test.dart | 37 ++++++++++++++- 4 files changed, 97 insertions(+), 11 deletions(-) diff --git a/docs/user-guide.md b/docs/user-guide.md index bba0f411..4b0dfdd1 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -46,6 +46,11 @@ High-level feature depth varies by database type. PostgreSQL, MySQL, and SQLite ### Result row caps (SQL workspaces) -Preferences → **Max rows in results** caps how many rows the grid loads. For **PostgreSQL** and **SQLite**, ad-hoc `SELECT` / `WITH` / `VALUES` without an author `LIMIT` get a `LIMIT` injected before execution so the engine does not materialize an unbounded result. Queries that already include `LIMIT`, and non-SELECT statements (`INSERT`, `PRAGMA`, …), are left unchanged; the UI may still truncate the displayed grid as a fallback. +Preferences → **Max rows in results** caps how many rows the grid loads. For **PostgreSQL** and **SQLite**, ad-hoc `SELECT` / `WITH` / `VALUES` are bounded **before** execution: + +- No author `LIMIT` → a `LIMIT` equal to the preference is injected. +- Author `LIMIT` / `LIMIT ALL` / `FETCH FIRST n ROWS ONLY` larger than the preference → clamped down to the preference (OFFSET kept when present). + +Queries that already use a smaller `LIMIT`, and non-SELECT statements (`INSERT`, `PRAGMA`, …), are left unchanged. The UI may still truncate the displayed grid as a fallback. **MySQL** SQL workspace streams rows and stops at the cap client-side. For troubleshooting build/run issues, see the main [README.md](../README.md). diff --git a/lib/core/database/sql_limit.dart b/lib/core/database/sql_limit.dart index 45a81a8c..40211e75 100644 --- a/lib/core/database/sql_limit.dart +++ b/lib/core/database/sql_limit.dart @@ -15,13 +15,25 @@ String stripLeadingWhitespaceAndLineComments(String sql) { } } -/// Injects a `LIMIT` clause into a read-only query (`SELECT`, `WITH`, `VALUES`) -/// when it does not already contain `LIMIT`. +final _limitAll = RegExp(r'\bLIMIT\s+ALL\b', caseSensitive: false); +final _limitCount = RegExp( + r'\bLIMIT\s+(\d+)(\s+OFFSET\s+\d+)?', + caseSensitive: false, +); +final _fetchFirst = RegExp( + r'\bFETCH\s+(?:FIRST|NEXT)\s+(\d+)\s+ROWS?\s+ONLY\b', + caseSensitive: false, +); + +/// Injects or clamps a `LIMIT` on read-only queries (`SELECT`, `WITH`, `VALUES`). /// -/// Existing `LIMIT` is left unchanged (caller may still apply a client-side -/// display cap). Non-select statements are returned as-is. +/// - No `LIMIT` / `FETCH … ONLY` → appends `LIMIT [limit]`. +/// - `LIMIT ALL` → replaced with `LIMIT [limit]`. +/// - `LIMIT n [OFFSET m]` where `n > limit` → clamped to [limit]. +/// - `FETCH FIRST/NEXT n ROWS ONLY` where `n > limit` → clamped. +/// - Non-select statements are returned unchanged. /// -/// Trailing semicolons are preserved after the injected clause. +/// Trailing semicolons are preserved after an injected clause. String injectSqlLimit(String sql, int limit) { if (limit <= 0) return sql; @@ -36,9 +48,38 @@ String injectSqlLimit(String sql, int limit) { return sql; } - // Already bounded by the author (may still exceed UI cap — see clamp issue). - final hasLimit = RegExp(r'\bLIMIT\b', caseSensitive: false).hasMatch(sql); - if (hasLimit) { + if (_limitAll.hasMatch(sql)) { + return sql.replaceFirst(_limitAll, 'LIMIT $limit'); + } + + final limitMatch = _limitCount.firstMatch(sql); + if (limitMatch != null) { + final existing = int.tryParse(limitMatch.group(1)!); + if (existing == null || existing <= limit) { + return sql; + } + final offsetPart = limitMatch.group(2) ?? ''; + return sql.replaceFirst( + limitMatch.group(0)!, + 'LIMIT $limit$offsetPart', + ); + } + + final fetchMatch = _fetchFirst.firstMatch(sql); + if (fetchMatch != null) { + final existing = int.tryParse(fetchMatch.group(1)!); + if (existing == null || existing <= limit) { + return sql; + } + return sql.replaceFirst( + fetchMatch.group(0)!, + 'FETCH FIRST $limit ROWS ONLY', + ); + } + + if (RegExp(r'\bLIMIT\b', caseSensitive: false).hasMatch(sql) || + RegExp(r'\bFETCH\b', caseSensitive: false).hasMatch(sql)) { + // Unrecognized LIMIT/FETCH shape — leave unchanged. return sql; } diff --git a/test/core/database/postgres_sql_test.dart b/test/core/database/postgres_sql_test.dart index 0ad57b89..85bf320a 100644 --- a/test/core/database/postgres_sql_test.dart +++ b/test/core/database/postgres_sql_test.dart @@ -95,13 +95,18 @@ void main() { 'SELECT * FROM users\nLIMIT 5000;;'); }); - test('does not append LIMIT if LIMIT already exists', () { + test('does not append LIMIT if LIMIT already within cap', () { expect(injectSqlLimit('SELECT * FROM users LIMIT 10', 5000), 'SELECT * FROM users LIMIT 10'); expect(injectSqlLimit('SELECT * FROM users limit 10;', 5000), 'SELECT * FROM users limit 10;'); }); + test('clamps oversized LIMIT', () { + expect(injectSqlLimit('SELECT * FROM users LIMIT 999999', 5000), + 'SELECT * FROM users LIMIT 5000'); + }); + test('does not modify non-select/non-read queries', () { expect(injectSqlLimit('INSERT INTO users VALUES (1)', 5000), 'INSERT INTO users VALUES (1)'); diff --git a/test/core/database/sql_limit_test.dart b/test/core/database/sql_limit_test.dart index dd3468f5..24d586a0 100644 --- a/test/core/database/sql_limit_test.dart +++ b/test/core/database/sql_limit_test.dart @@ -25,7 +25,7 @@ void main() { ); }); - test('does not append LIMIT if LIMIT already exists', () { + test('does not append LIMIT if LIMIT already exists and within cap', () { expect( injectSqlLimit('SELECT * FROM users LIMIT 10', 5000), 'SELECT * FROM users LIMIT 10', @@ -36,6 +36,41 @@ void main() { ); }); + test('clamps LIMIT larger than cap', () { + expect( + injectSqlLimit('SELECT * FROM users LIMIT 999999', 5000), + 'SELECT * FROM users LIMIT 5000', + ); + expect( + injectSqlLimit('SELECT * FROM users LIMIT 100000 OFFSET 20;', 1000), + 'SELECT * FROM users LIMIT 1000 OFFSET 20;', + ); + }); + + test('replaces LIMIT ALL with cap', () { + expect( + injectSqlLimit('SELECT * FROM users LIMIT ALL', 5000), + 'SELECT * FROM users LIMIT 5000', + ); + }); + + test('clamps FETCH FIRST n ROWS ONLY', () { + expect( + injectSqlLimit( + 'SELECT * FROM users FETCH FIRST 100000 ROWS ONLY', + 5000, + ), + 'SELECT * FROM users FETCH FIRST 5000 ROWS ONLY', + ); + expect( + injectSqlLimit( + 'SELECT * FROM users FETCH FIRST 10 ROWS ONLY', + 5000, + ), + 'SELECT * FROM users FETCH FIRST 10 ROWS ONLY', + ); + }); + test('does not modify non-select/non-read queries', () { expect( injectSqlLimit('INSERT INTO users VALUES (1)', 5000),