From 2f19f9b6014141acc5ff3910c45c0b72b5dc0d0c Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 6 Jul 2026 11:48:46 -0700 Subject: [PATCH 1/2] perf(channel): bound cleanup candidate query in SQL instead of driver maxrows DatabaseAdapter.cleanup() bounded passes previously capped the candidate SELECT with the driver-level maxrows option, so the database still materialized the entire expired backlog and the client truncated it. The maxRows bound is now pushed into dialect SQL via a new $applyRowBound() helper (SELECT TOP n on SQL Server, FETCH FIRST n ROWS ONLY on Oracle, LIMIT n for mysql/postgresql/sqlite/h2/default) so the database performs a true index-assisted top-n read against idx_wevents_cleanup. The bound is hardened with Int() before interpolation and the driver-level maxrows option is kept as belt-and-braces. The two-step SELECT-then-DELETE-by-id shape is unchanged. Unit specs cover the three dialect shapes plus integer hardening and the non-positive no-op. Signed-off-by: Peter Amiri --- ...annel-cleanup-sql-row-bound.performance.md | 1 + vendor/wheels/channel/DatabaseAdapter.cfc | 50 +++++++++++++++-- .../specs/channel/DatabaseAdapterSpec.cfc | 54 +++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 changelog.d/channel-cleanup-sql-row-bound.performance.md diff --git a/changelog.d/channel-cleanup-sql-row-bound.performance.md b/changelog.d/channel-cleanup-sql-row-bound.performance.md new file mode 100644 index 0000000000..d6f37f504d --- /dev/null +++ b/changelog.d/channel-cleanup-sql-row-bound.performance.md @@ -0,0 +1 @@ +- `wheels.channel.DatabaseAdapter.cleanup()` now pushes the `maxRows` bound into dialect SQL (`SELECT TOP n` on SQL Server, `FETCH FIRST n ROWS ONLY` on Oracle, `LIMIT n` everywhere else) via the new `$applyRowBound()` helper, so bounded retention passes do an index-assisted top-n read instead of materializing the whole expired backlog and truncating it client-side. The driver-level `maxrows` option is kept as belt-and-braces. diff --git a/vendor/wheels/channel/DatabaseAdapter.cfc b/vendor/wheels/channel/DatabaseAdapter.cfc index 6c047906f0..ecc06e0260 100644 --- a/vendor/wheels/channel/DatabaseAdapter.cfc +++ b/vendor/wheels/channel/DatabaseAdapter.cfc @@ -162,13 +162,22 @@ component { if (arguments.maxRows > 0) { // Bounded pass: select the oldest expired ids first, then delete only // those, so the DELETE (the lock-holding statement) stays small and - // indexed even when a large backlog has accumulated + // indexed even when a large backlog has accumulated. The row bound is + // pushed into dialect SQL (TOP / FETCH FIRST / LIMIT) so the database + // does an index-assisted top-n read instead of materializing the whole + // expired backlog and truncating it client-side. The driver-level + // maxrows option stays on as belt-and-braces. + local.candidateSql = $applyRowBound( + sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + dbType = $detectDatabaseType(), + maxRows = arguments.maxRows + ); local.candidates = queryExecute( - "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + local.candidateSql, { cutoff: {value: local.cutoff, cfsqltype: "cf_sql_timestamp"} }, - {datasource: variables.$datasource, maxrows: arguments.maxRows} + {datasource: variables.$datasource, maxrows: Int(arguments.maxRows)} ); if (local.candidates.recordCount == 0) { return 0; @@ -323,4 +332,39 @@ component { return "default"; } + /** + * Rewrite a candidate SELECT so the row bound is applied in dialect SQL and the + * database can do an index-assisted top-n read instead of materializing every + * matching row and relying on client-side truncation. + * + * - sqlserver: SELECT TOP n ... + * - oracle: ... FETCH FIRST n ROWS ONLY + * - mysql / postgresql / sqlite / h2 / default: ... LIMIT n + * + * The bound is hardened with Int() so only a plain integer is ever interpolated + * into the SQL string. A bound of zero or less returns the statement unchanged. + * Public with $ prefix (internal naming convention) so it stays unit-testable. + * + * @sqlText The SELECT statement to bound. + * @dbType Database type as returned by $detectDatabaseType(). + * @maxRows Maximum number of rows the statement may return. + */ + public string function $applyRowBound( + required string sqlText, + required string dbType, + required numeric maxRows + ) { + local.bound = Int(arguments.maxRows); + if (local.bound <= 0) { + return arguments.sqlText; + } + if (arguments.dbType == "sqlserver") { + return ReplaceNoCase(arguments.sqlText, "SELECT ", "SELECT TOP #local.bound# ", "one"); + } + if (arguments.dbType == "oracle") { + return arguments.sqlText & " FETCH FIRST #local.bound# ROWS ONLY"; + } + return arguments.sqlText & " LIMIT #local.bound#"; + } + } diff --git a/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc b/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc index 42a77abe76..21692d28b8 100644 --- a/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc +++ b/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc @@ -222,6 +222,60 @@ component extends="wheels.WheelsTest" { expect(remaining.recordCount).toBe(3); }); + it("$applyRowBound rewrites the SELECT with TOP for sqlserver", function() { + var bounded = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + dbType = "sqlserver", + maxRows = 25 + ); + expect(bounded).toBe( + "SELECT TOP 25 id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC" + ); + }); + + it("$applyRowBound appends FETCH FIRST for oracle", function() { + var bounded = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + dbType = "oracle", + maxRows = 25 + ); + expect(bounded).toBe( + "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC FETCH FIRST 25 ROWS ONLY" + ); + }); + + it("$applyRowBound appends LIMIT for every other dialect", function() { + var dialects = ["mysql", "postgresql", "sqlite", "h2", "default"]; + for (var dialect in dialects) { + var bounded = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + dbType = dialect, + maxRows = 25 + ); + expect(bounded).toBe( + "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC LIMIT 25" + ); + } + }); + + it("$applyRowBound hardens the bound to an integer", function() { + var bounded = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events", + dbType = "mysql", + maxRows = 7.9 + ); + expect(bounded).toBe("SELECT id FROM wheels_events LIMIT 7"); + }); + + it("$applyRowBound leaves the statement unchanged for a non-positive bound", function() { + var unbounded = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events", + dbType = "mysql", + maxRows = 0 + ); + expect(unbounded).toBe("SELECT id FROM wheels_events"); + }); + it("auto-creates wheels_events table on first use", function() { // The table should already exist from previous tests, // but verify we can query it From e23f55ead52586ad4240a75108031aaef1efc4a0 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 6 Jul 2026 12:42:08 -0700 Subject: [PATCH 2/2] fix(channel): leave unknown dialects unbounded in SQL so cleanup survives cfdbinfo failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding: the 'default' fallback appended LIMIT n, which is invalid on SQL Server/Oracle — exactly where dialect detection matters. When cfdbinfo fails on those engines cleanup() would throw, get swallowed, and return 0 forever. Unknown dialects now keep the statement unchanged; the retained driver-level maxrows option still bounds the resultset on every engine. Signed-off-by: Peter Amiri --- vendor/wheels/channel/DatabaseAdapter.cfc | 13 ++++++++++-- .../specs/channel/DatabaseAdapterSpec.cfc | 21 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/vendor/wheels/channel/DatabaseAdapter.cfc b/vendor/wheels/channel/DatabaseAdapter.cfc index ecc06e0260..b8353bfdb2 100644 --- a/vendor/wheels/channel/DatabaseAdapter.cfc +++ b/vendor/wheels/channel/DatabaseAdapter.cfc @@ -339,7 +339,11 @@ component { * * - sqlserver: SELECT TOP n ... * - oracle: ... FETCH FIRST n ROWS ONLY - * - mysql / postgresql / sqlite / h2 / default: ... LIMIT n + * - mysql / postgresql / sqlite / h2: ... LIMIT n + * - anything else (incl. "default" when $detectDatabaseType() falls back on a + * cfdbinfo failure): statement UNCHANGED — appending LIMIT would be a syntax + * error on SQL Server/Oracle, and the caller keeps the driver-level maxrows + * option on the query, which still bounds the resultset on every engine. * * The bound is hardened with Int() so only a plain integer is ever interpolated * into the SQL string. A bound of zero or less returns the statement unchanged. @@ -364,7 +368,12 @@ component { if (arguments.dbType == "oracle") { return arguments.sqlText & " FETCH FIRST #local.bound# ROWS ONLY"; } - return arguments.sqlText & " LIMIT #local.bound#"; + if (ListFindNoCase("mysql,postgresql,sqlite,h2", arguments.dbType)) { + return arguments.sqlText & " LIMIT #local.bound#"; + } + // Unknown dialect (incl. the "default" cfdbinfo-failure fallback): leave the + // statement alone rather than risk invalid syntax; driver maxrows bounds it. + return arguments.sqlText; } } diff --git a/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc b/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc index 21692d28b8..bc2c23e75b 100644 --- a/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc +++ b/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc @@ -244,8 +244,8 @@ component extends="wheels.WheelsTest" { ); }); - it("$applyRowBound appends LIMIT for every other dialect", function() { - var dialects = ["mysql", "postgresql", "sqlite", "h2", "default"]; + it("$applyRowBound appends LIMIT for the explicit LIMIT dialects", function() { + var dialects = ["mysql", "postgresql", "sqlite", "h2"]; for (var dialect in dialects) { var bounded = adapter.$applyRowBound( sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", @@ -258,6 +258,23 @@ component extends="wheels.WheelsTest" { } }); + it("$applyRowBound leaves unknown dialects unchanged so driver maxrows stays the bound", function() { + // "default" is what $detectDatabaseType() returns when cfdbinfo fails — + // appending LIMIT there would be a syntax error on SQL Server/Oracle, + // silently breaking cleanup() on the engines that need dialect handling. + var dialects = ["default", "informix"]; + for (var dialect in dialects) { + var unchanged = adapter.$applyRowBound( + sqlText = "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC", + dbType = dialect, + maxRows = 25 + ); + expect(unchanged).toBe( + "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC" + ); + } + }); + it("$applyRowBound hardens the bound to an integer", function() { var bounded = adapter.$applyRowBound( sqlText = "SELECT id FROM wheels_events",