Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database getNumRows() #109

Closed
wants to merge 6 commits into from
Closed

Database getNumRows() #109

wants to merge 6 commits into from

Conversation

turtuga
Copy link
Contributor

@turtuga turtuga commented Jun 10, 2016

The number of rows returned by the query. Make sure to call the method using your query result object:

$query = $db->query('SELECT * FROM my_table');

echo $query->getNumRows();

$mysql = \Config\Database::connect('mysql', true);

$num_rows = $mysql->query("SELECT * FROM teste")->numRows();
@jim-parry
Copy link
Contributor

Is this a proposed enhancement to ResultInterface? If so, that would be appropriate as a description.

Also, if that is the case, it would need changes in ResultInterface, along with the related user guide and unit testing changes.

Also, it should follow the style of the other interface methods, namely be called getNumRows.

@turtuga
Copy link
Contributor Author

turtuga commented Jun 11, 2016

I have merged the other branch please close other PR please. can you explain me how unit testing works please??

@turtuga turtuga changed the title NumRows Mysql Database getNumRows() Jun 11, 2016
@lonnieezell
Copy link
Member

I had left the numRows out on purpose. We can always discuss whether to provide it or not but we had been discouraging its use for the last couple of years and writing it out of the user guide examples due to performance and memory issues on several platforms.

@turtuga
Copy link
Contributor Author

turtuga commented Jun 11, 2016

and how validate if row with certain id exists??

@lonnieezell
Copy link
Member

lonnieezell commented Jun 11, 2016

All getResult methods will return an empty array and all getRow methods return null if the record doesn't exist.

If (! is_null($query->getRow()) { }

@turtuga
Copy link
Contributor Author

turtuga commented Jun 11, 2016

make sense 👍

@lonnieezell
Copy link
Member

Since I don't see any discussion about whether we need this or not I'm going to assume that we're good, and close it.

@turtuga turtuga deleted the patch-2 branch June 23, 2016 14:22
@sneakyimp
Copy link
Contributor

sneakyimp commented Dec 29, 2020

I suspect that the vast majority of CodeIgniter projects use one of the DBMS modules that have a num_rows function (MySQLi, Postgres, MSSQL). I strongly believe that CodeIgniter 4 should expose a Result->numRows method for a variety of reasons which I've enumerated on the CodeIgniter forum.

I would add to these points the fact that my existing CI3 project uses query->num_rows in no less than a hundred places and that this function was attached not to a query build or db class, but to the query result object. I hope that the CI4 devs will consider the difficulty of CI3 -> CI4 migration and add this very convenient and efficient function for the popular DBMSes that support it.

EDIT: it also occurred to me that there is no countAllResults function available if one just used db->query or db->simpleQuery instead of the QueryBuilder. Please restore this vital function to the database Result classes that natively support a num_rows function: MySQLi, Postgres, SQLSRV.

My post from the CI forums follows:

Unless I'm missing something, it's not always "a simple count()". In some cases, queries can be enormously complicated with many tables joined and distinct or group by or sum/avg/max specified. The db method countAllResults does seem to take a lot of this into account, but the extra query that must be run also appears to obliterate any prior query results which might exist in the Builder object. And I'm not sure, but mightn't be some (probably very small) risk of a name collision between the numrows used as an alias for the COUNT(*) in the other query.

Perhaps most critically, this countAllResults function clearly contains more PHP code than a single mysqli_num_rows, sqlsrv_num_rows, or pg_num_rows function call and it requires another query to be run on the db server. In my experience, a db server is quite likely to be your bottleneck on a busy server. If the PHP client has some simple built-in function to return the number of rows corresponding to a query, why not use it? The coding effort to expose this function for the most popular database engines is probably trivial, and you can just throw an exception ("use countAllResults instead!") if the DBMS does not support such a function. I could be dead wrong, but I expect the vast majority of CI developers are using MySQLi, MS SQL, or PostGres, and all of those support this function.

To bolster my point, consider the PHP source code for the mysqli_num_rows function. It goes through a few macros but I think they end up just returning the value from a struct.

@lonnieezell
Copy link
Member

We would happily accept a PR for this if you'd care to implement it.

@sneakyimp
Copy link
Contributor

sneakyimp commented Dec 31, 2020

@lonnieezell should I fork the develop branch? Or some other branch?

EDIT: I see here that pull requests should use the develop branch:

Pull requests should be from a feature branch in the contributor's fork of the repository to the develop branch of the project repository

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.

None yet

4 participants