Ticket #1117 (fixed enhancement)
avoid $db->num_rows()
- Created: 2018-07-11 13:30:53
- Reported by: root_hippyru
- Assigned to: Franz
- Milestone: 1.6
- Component: database
- Priority: normal
One of the difficulties with supporting SQLite3 or PDO in fluxbb is that corresponding PHP API does not provide a way to get the number of rows in the result set without fetching all rows first. Yet fluxbb code assumes that such functionality is available and freely uses $db->num_rows() throughout the code.
However in all cases the reliance on $db->num_rows() can be eliminated.
Most of the calls to $db->num_rows() just checks if there are any row in the result. Such cases can be replaced with a new driver method has_rows() that can be implemented efficiently with SQLite3/PDO.
In other cases the code uses num_rows() as a stop condition fir loops fetching all results. This can be trivially replaced by loops that fetch results until the fetching methods return false.
The remaining cases uses num_rows($result) as the sole purpose of the result. This can be replaced by doing proper SELECT COUNT(*) calls. This may even improove the performance as the DB does not need to send the result set.
The last case is the check in moderate.php "$db->num_rows($result) < 2". But even here although convinience, num_rows is non-essentail and can be replaced by trying to fetch the 2 rows first and then fetching the rest.
History
Franz 2018-07-14 22:01:05
Those are great suggestions. This is one of the main reasons why the SQLite3 driver was never released.
We resorted to an ugly hack instead.
Feel free to send PRs with replacements for each of these, probably in multiple steps: First, add a new has_rows() - or, my preference, exists() - method for each driver. Then, replace the occurrences of num_rows(), optimally one PR per category you described.
Visman 2018-09-02 07:26:54
A new method (has_rows()) is not needed.
https://github.com/MioVisman/FluxBB_by_ … pe=Commits
Franz 2018-09-02 18:21:52
I've already got most of this sorted on a local branch. Thanks for the suggestion.
Franz 2018-11-25 13:09:23
- Owner set to Franz.
- Status changed from open to fixed.
Commit 9891ce1 to fluxbb 1.6-next
Fraesse 2018-11-30 09:05:36
for older versions, it could be replaced with:
function num_rows($query_id)
{
return mysqli_num_rows($query_id) > 0;
}
this is mysqli
Franz 2018-11-30 11:05:21
Which older versions do you mean? The 1.5 branch has the method, 1.6 does not need it any more (and it has been removed).