Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.5.0b4

Bug #11571 Bug in numRows() function
Submitted: 2007-07-09 09:13 UTC Modified: 2008-05-03 07:34 UTC
From: tom057 Assigned: quipo
Status: Closed Package: MDB2_Driver_mssql (version 1.2.1)
PHP Version: 5.2.1 OS: Windows XP
Roadmaps: (Not assigned)    
Subscription  


 [2007-07-09 09:13 UTC] tom057 (Lallement Thomas)
Description: ------------ When using limit, numRows() returns wrong values. Test script: --------------- $sql = "SELECT * FROM JOURNALISTS"; $result = $db->query($sql); $db->setLimit(5, 0); $nb_row = $result->numRows() -> 1 (it would be 5) I resolved the problem while commenting in numRows(): if ($this->limit) { $rows -= $this->limit; if ($rows < 0) { $rows = 0; } } Expected result: ---------------- $nb_row -> 5 Actual result: -------------- $nb_row -> 1

Comments

 [2007-07-09 09:45 UTC] wiesemann (Mark Wiesemann)
(package selection corrected)
 [2007-07-09 11:34 UTC] aharvey (Adam Harvey)
Which database driver are you using?
 [2007-07-10 02:43 UTC] tom057 (Lallement Thomas)
I used the mssql driver. For which date is planned the stable release?
 [2007-07-10 03:24 UTC] aharvey (Adam Harvey)
Thanks for that. I do releases at the end of each month, generally speaking.
 [2007-07-10 09:12 UTC] aharvey (Adam Harvey)
I can't reproduce this. As far as I can tell, both through the automated regression tests (which do cover this) and writing a couple of simple test scripts of my own, DB 1.7.8 and later all handle numRows() calculation correctly for mssql limit queries. Furthermore, I note that your example code is calling setLimit, a method which doesn't exist in PEAR DB. As such, I have to assume that you're not using a standard install of PEAR DB, and therefore can't really help.
 [2007-07-10 09:35 UTC] tom057 (Lallement Thomas)
I am so sorry, I omitted to inform the package which was concerned by this bug. I've just change the default selected package in list from "DB" to "MDB2". In fact to be accurate, I used Pear MDB2 with a mssql connection throught MDB2_Driver_mssql in 1.2.1 version (the last release). I use a standart version of the package. Once again sorry and thanks for your reactivity.
 [2007-07-10 10:27 UTC] davidc (David Coallier)
Will check this
 [2007-08-09 07:48 UTC] tom057 (Lallement Thomas)
I read again my post and I have seen in my sample setLimit must be placed before query as you can see: ------------------------------------------------ $sql = "SELECT * FROM JOURNALISTS"; // 50 records $db->setLimit(5, 0); $result = $db->query($sql); $nb_row = $result->numRows() -> 1 (it would be 5) ------------------------------------------------ Have you check? In fact if you compare the function of mssql driver for DB and the one of MDB2, you can notice that the implementation is different. In the driver of MDB2, there is the following lines: if ($this->limit) { $rows -= $this->limit; if ($rows < 0) { $rows = 0; } } However, I think you shouldn't check the limit here because numRows is call from the result of query. Thanks
 [2007-08-11 11:56 UTC] quipo (Lorenzo Alberton)
Yep, DB and MDB2 differ in this regard. setLimit() must be called BEFORE running the query. This is documented in the manual: http://pear.php.net/manual/en/package.database.mdb2.intro-query.php#package.database.mdb2.intro-query.setlimit
 [2007-08-13 03:47 UTC] tom057 (Lallement Thomas)
However, even if I call setLimit() before running query, numRows() return bad result. Please, try to implement the sample code I write: $sql = "SELECT * FROM JOURNALISTS"; // 50 records $db->setLimit(5, 0); $result = $db->query($sql); $nb_row = $result->numRows(); echo "NB RESULT = ".$nb_row; ------------- Result --------------- NB RESULT = 1 ------------------------------------ In this case $nb_row would be equal to 5 rather than 1...
 [2007-08-13 05:45 UTC] quipo (Lorenzo Alberton)
Ah, sorry, you're right, I was testing it with the other drivers and didn't notice that the mssql driver is indeed buggy. Will check ASAP.
 [2007-10-05 03:19 UTC] tom057 (Lallement Thomas)
Do you have any problem to fix this bug? I think you just have to comment the following lines in numRows of MDB2_Driver_mssql: if ($this->limit) { $rows -= $this->limit; if ($rows < 0) { $rows = 0; } } Do you planned to make a new release? In how long?
 [2007-11-13 18:03 UTC] mrsocks (Max Arbos)
I am having issues with this. It seems to be that if there are fewer results than the LIMIT attribute, then 'numRows' returns as '0' What needs to be done in the package to fix this reliably? Thanks.
 [2007-11-13 18:35 UTC] mrsocks (Max Arbos)
After looking further into this, what is the purpose of: if ($this->limit) { $rows -= $this->limit; if ($rows < 0) { $rows = 0; } } --------------- It looks like an attempt to stop negative limit values? In the current metod, no matter what the limit it set to, it changes to value of the $rows value to the limit. Maybe there is something in the setLimit funcion that may be causing this. I set that clip to: // if ($this->limit) { // $rows -= $this->limit; if ($this->limit <= 0) { $rows = 0; } // } and I get all rows returned, not the limit. Hopefully this will help shed some light on this issue and this package. .... or am I totally off? thanks.
 [2007-11-13 18:41 UTC] quipo (Lorenzo Alberton)
David, have you had a chance to look at this one?
 [2007-11-18 12:52 UTC] quipo (Lorenzo Alberton)
OK, I think I found the real problem, eventually. Can you guys fetch the CVS version of the package and tell me if it's fixed for you too? If not, please reopen the bug report. Thanks.
 [2008-04-22 09:28 UTC] luenne (Janis Luenne)
I still see an issue with this and think this bug has to be reopened: If there are less results than $this->limit, then numRows() returns 0 and not the actual number of rows. This is extremely painful if you use Structures_DataGrid and bind it to MDB2 with mssql. As long as your query returns more rows than specified in $this->limit, everything works fine, but if it returns less rows, the following part of the numRows() function calculates the wrong rowcount: Extraction from mssql.php latest stable (1.2.1): if ($this->limit) { $rows -= $this->limit; if ($rows < 0) { $rows = 0; } } Example: $rows is initially 5, $this->limit is 20, $this->offset is 0: $rows becomes -15 (5-20), is then lesser than 0 and therefore set to 0 even though there are actually 5 results. The extraction above was changed in the latest beta (1.3.0b1) to: if ($this->limit) { $rows -= $this->limit -1 + $this->offset; if ($rows < 0) { $rows = 0; } } Unfortunately it still returns an incorrect result if $rows is lesser than $this->limit. Example: $rows is initially 5, $this->limit is 20, $this->offset is 0: $rows becomes -16 (5-20-1+0), is then lesser than 0 and therefore set to 0 even though there are actually 5 results. Suggestion: If you change it like this, it may be used irrespective of whether or not $this->limit is lesser than $rows: if ($this->limit) { //$rows -= $this->limit -1 + $this->offset; $rows -= $this->offset; if ($rows < 0) { $rows = 0; } } Example: $rows is initially 5, $this->limit is 20, $this->offset is 0: $rows becomes 5 (5-0), is then greater than 0 and therefore stays 5 as desired. Another Example: $rows is initially 60, $this->limit is 20, $this->offset is 40: $rows becomes 20 (60-45), is then greater than 0 and therefore stays 20 as desired.
 [2008-05-03 07:34 UTC] quipo (Lorenzo Alberton)
@Janis: thanks for your analysis. Unfortunately what you suggest isn't correct yet. If $rows is initially 7, $this->limit is 2 and $this->offset is 3: $rows becomes 4 (7-3), is then greater than 0 and therefore stays 4, but it's also greater than $this->limit, which is wrong. I've committed this patch instead: ================================= if ($this->limit) { $rows -= $this->offset; if ($rows > $this->limit) { $rows = $this->limit; } if ($rows < 0) { $rows = 0; } } ================================= Please test it and reopen the bug report if you find any other flaw.