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..b8353bfdb2 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,48 @@ 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: ... 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. + * 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"; + } + 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 42a77abe76..bc2c23e75b 100644 --- a/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc +++ b/vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc @@ -222,6 +222,77 @@ 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 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", + dbType = dialect, + maxRows = 25 + ); + expect(bounded).toBe( + "SELECT id FROM wheels_events WHERE createdAt < :cutoff ORDER BY createdAt ASC LIMIT 25" + ); + } + }); + + 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", + 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