Skip to content

Fix FOUND_ROWS() without SQL_CALC_FOUND_ROWS - #533

Merged
JanJakes merged 2 commits into
WordPress:trunkfrom
Nandan108:fix/found-rows-above-nine
Oct 14, 2026
Merged

JanJakes merged 2 commits into
WordPress:trunkfrom
Nandan108:fix/found-rows-above-nine

Conversation

@Nandan108

@Nandan108 Nandan108 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After a SELECT without SQL_CALC_FOUND_ROWS, or after a SHOW statement, FOUND_ROWS() returns only the first digit of the row count:

-- Table "t" has 25 rows.
SELECT id FROM t;
SELECT FOUND_ROWS();
-- '2'   (MySQL: 25)

The count of the previous query is read with fetchColumn()[0], which takes the first character of a stringified count. Without stringified fetches, indexing the integer count returns null, so the result is 0 and PHP emits a warning.

SELECT SQL_CALC_FOUND_ROWS ... is not affected; it uses a different path.

Fix

The fetched count is used directly.

Tests

testFoundRowsWithoutSqlCalcFoundRowsAboveNine checks FOUND_ROWS() after a 12-row SELECT and after SHOW TABLES with 13 tables, which covers both code paths. Each assertion fails on trunk. The existing testFoundRowsWithoutSqlCalcFoundRows only uses counts below ten.

packages/mysql-on-sqlite: 1044 tests pass (PHP 8.3, SQLite 3.45.1); PHPCS is clean.

Use of AI

This change was prepared with Claude Code (Claude Opus 5.5). I reviewed the code, the tests, and this description, and I take responsibility for the change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Corrected FOUND_ROWS() results for queries returning counts above nine, including when the database returns fetched values as strings.
  • Tests

    • Expanded coverage for row counts from unrestricted SELECT queries and metadata commands, including SHOW TABLES with multiple tables. Verified results with both database fetch settings.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 46106fd8-9049-49d8-9e92-c056de8a6c8f
📥 Commits

Reviewing files that changed from the base of the PR and between d56c4a1 and 3bed95e.

📒 Files selected for processing: 2
  • packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
  • packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

The change updates how query-backed FOUND_ROWS() results are read. Tests cover both fetch-stringification settings, a 12-row SELECT, and SHOW TABLES with 11 tables.

Changes: FOUND_ROWS() result handling

Layer / File(s) Summary
Read and verify query-backed counts
packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
FOUND_ROWS() now casts the scalar returned by fetchColumn() to an integer. Tests check the SELECT count with both fetch-stringification settings and check the SHOW TABLES count after creating additional tables.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: janjakes

Merge Risk: ⚪ Minimal · up to 3bed9

The change reads the query-backed count directly, and the tests cover SELECT and SHOW results under both fetch modes. No actionable merge risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: fixing FOUND_ROWS() for counts above nine when SQL_CALC_FOUND_ROWS is not used.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 PHPStan (2.2.14)

PHPStan reported 1 non-file analysis error(s); analysis is incomplete

🔧 PHPMD (2.15.0)
packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

PHPMD could not process this file (exit code 255): PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in phar:///usr/bin/phpmd/vendor/pdepend/pdepend/src/main/php/PDepend/Util/Cache/Driver/FileCacheDriver.php on line 209

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Nandan108 and others added 2 commits October 14, 2026 16:09
After a SELECT without SQL_CALC_FOUND_ROWS or after a SHOW statement,
FOUND_ROWS() counts the rows of the previous query. The count was read
with fetchColumn()[0], which takes the first character of a stringified
count, so FOUND_ROWS() returned "1" for 12 rows. Without stringified
fetches, indexing the integer count returned null, and the result was 0.

Use the fetched count directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fold the separate above-nine test into testFoundRowsWithoutSqlCalcFoundRows.
The SELECT and SHOW TABLES checks now return 12 and 11 rows, which covers
both the query and the query-with-params paths. The test no longer counts
the setUp fixture tables, as the existing test drops them.

Run the test with stringified fetches on and off. Without stringification,
the old code returned 0 and emitted a warning.
@JanJakes
JanJakes force-pushed the fix/found-rows-above-nine branch from d56c4a1 to 3bed95e Compare October 14, 2026 14:12
@JanJakes

Copy link
Copy Markdown
Member

Thanks again, @Nandan108! Good catch, and a clean fix. I rebased the branch on trunk and made a few small changes to the tests.

@JanJakes
JanJakes merged commit 81d6afa into WordPress:trunk Oct 14, 2026
24 checks passed
@JanJakes JanJakes changed the title Fix FOUND_ROWS() for more than nine rows without SQL_CALC_FOUND_ROWS Fix FOUND_ROWS() for more than nine rows without SQL_CALC_FOUND_ROWS Oct 14, 2026
@JanJakes JanJakes changed the title Fix FOUND_ROWS() for more than nine rows without SQL_CALC_FOUND_ROWS Fix FOUND_ROWS() without SQL_CALC_FOUND_ROWS Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants