Fork me on GitHub
Subscribe 3

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.

Franz 2018-07-18 09:20:14

  • Milestone set to 1.6.

Franz 2018-07-18 09:44:44

Related to #1044.

Franz 2018-07-23 22:42:19

Commit 06a4ad6 to fluxbb 1.6-next

Implement a new DBLayer::has_rows() method

This can be used in most places where we currently call num_rows(),
which is not (reasonably easily) implementable for (future) SQLite
or PDO adapters.

Refs #1117.

Franz 2018-07-28 21:08:31

Commit 02512bd to fluxbb 1.6-next

Use new has_rows() method where possible

Refs #1117.

Franz 2018-07-28 21:08:32

Commit 20bb359 to fluxbb 1.6-next

Mark num_rows() method as deprecated

Refs #1117.

Franz 2018-07-28 21:09:47

Commit 3ba2bf7 to fluxbb 1.6-next

Use other query methods instead of num_rows()

We do not need to use num_rows() for iteration purposes.

Refs #1117.

Franz 2018-07-28 21:23:11

Commit 1aef372 to fluxbb 1.6-next

Let the database count

Refs #1117.

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:07:13

Commit 6bdf4f5 to fluxbb 1.6-next

Avoid using num_rows(), count in PHP

Refs #1117.

Franz 2018-11-25 13:09:23

  • Owner set to Franz.
  • Status changed from open to fixed.

Commit 9891ce1 to fluxbb 1.6-next

Remove deprecated num_rows() function

Fixes #1117.

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

Comment edited 1 times (Diff)

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).