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

Bug #9913 Incorrect count when using subqueries
Submitted: 2007-01-22 08:34 UTC Modified: 2007-03-11 09:35 UTC
From: HarrySchool at netscape dot net Assigned: wiesemann
Status: Closed Package: Structures_DataGrid_DataSource_MDB2 (version 0.1.5)
PHP Version: Irrelevant OS: Irrelevant
Roadmaps: (Not assigned)    
Subscription  


 [2007-01-22 08:34 UTC] HarrySchool at netscape dot net (Harry School)
Description: ------------ Counting the number of records for a query is done by replacing the columns by COUNT(*). This is done incorrectly when using subqueries. In function count() line 285 The following codelines are concerned: $query = preg_replace('#SELECT\s.*\sFROM#is', 'SELECT COUNT(*) FROM', $this->_query); Replacing this with $query = preg_replace('#SELECT\s.*?\sFROM#is', 'SELECT COUNT(*) FROM', $this->_query, 1); should cure the problem. Test script: --------------- $qry = 'select aa from bb where cc in select dd from ee;'; $pattern = '#SELECT\s.*\sFROM#is'; $query = preg_replace($pattern, 'SELECT COUNT(*) FROM', $qry); echo 'result: '.$query; Expected result: ---------------- The actual result is: $query = 'SELECT COUNT(*) FROM ee;' The correct result should be: $query = 'SELECT COUNT(*) FROM bb where cc in select dd from ee;' Actual result: -------------- The actual (wrong) result is: $query = 'SELECT COUNT(*) FROM ee;' In function count() line 285 The following codelines are concerned: $query = preg_replace('#SELECT\s.*\sFROM#is', 'SELECT COUNT(*) FROM', $this->_query); Replacing this with $query = preg_replace('#SELECT\s.*?\sFROM#is', 'SELECT COUNT(*) FROM', $this->_query, 1); should cure the problem.

Comments

 [2007-01-23 12:03 UTC] wiesemann (Mark Wiesemann)
You're right, the current replacement is to greedy. Your fix looks good. I'll take care of fixing the MDB2 and DBQuery drivers.
 [2007-01-24 14:57 UTC] wiesemann (Mark Wiesemann)
Thank you for your bug report. This issue has been fixed in the latest released version of the package, which you can download at http://pear.php.net/get/Structures_DataGrid_DataSource_MDB2
 [2007-01-25 11:46 UTC] HarrySchool at netscape dot net
Dear Mark, the fix in version 0.1.6 is partially correct. You forgot the question mark in the pattern: '#SELECT\s.*?\sFROM#is' Harry
 [2007-01-25 15:57 UTC] wiesemann (Mark Wiesemann)
> the fix in version 0.1.6 is partially correct. You forgot > the question mark in the pattern: '#SELECT\s.*?\sFROM#is' I had noted only the additional parameter. What is the purpose of this additional question mark? I don't see anything in the regexp that this question mark could be assigned to? Why isn't the avoidance of multiple replacements not enough?
 [2007-01-30 15:37 UTC] HarrySchool at netscape dot net
The question mark makes the * less greedy. Without the question mark everthing till the last FROM is replaced, whereas with questionmark only till the first FROM is replaced. The one (1) couses only one replacement. In the following example there could be two replacements because the select has two SELECt ... FROM combinations. Example: 'SELECT COUNT(*) FROM bb where cc in SELECT dd FROM ee;' = preg_replace('#SELECT\s.*?\sFROM#is', 'SELECT COUNT(*) FROM', 'SELECT aa FROM bb where cc in select dd from ee;', 1); 'SELECT COUNT(*) FROM ee;' = preg_replace('#SELECT\s.*\sFROM#is', 'SELECT COUNT(*) FROM', 'SELECT aa FROM bb where cc in select dd from ee;', 1);
 [2007-01-31 17:36 UTC] wiesemann (Mark Wiesemann)
Harry, thanks for the clarification. I'll take care of fixing my fix. (BTW: There was (IMHO) no real need to change the summary.)
 [2007-02-01 06:48 UTC] wiesemann (Mark Wiesemann)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.
 [2007-02-01 07:22 UTC] HarrySchool at netscape dot net
Mark, I had to fill in a summary, otherwise I could nog post a new comment. I had no idea that this would change the original summary. Thanks for fixing. Harry
 [2007-02-12 07:24 UTC] mail at coppermine-gallery dot net (Tom Atkinson)
This fix breaks previously working code. Example query: SELECT ticket_id, phone, ticket_time, ticket_status, amount, total_odds, ROUND(amount * total_odds, 2) AS poss_win, (SELECT COUNT(*) FROM bets WHERE ticketid = t.ticket_id) AS numbets FROM tickets AS t INNER JOIN subscribers AS s ON s.subscriber_id = t.owner WHERE approved = 'APPROVED' Expected result: SELECT COUNT(*) FROM tickets AS t INNER JOIN subscribers AS s ON s.subscriber_id = t.owner WHERE approved = 'APPROVED' Actual result: SELECT COUNT(*) FROM bets WHERE ticketid = t.ticket_id) AS numbets FROM tickets AS t INNER JOIN subscribers AS s ON s.subscriber_id = t.owner WHERE approved = 'APPROVED' Which is not valid SQL. 0.1.5 and 0.1.6 both give the expected result.
 [2007-02-12 14:18 UTC] HarrySchool at netscape dot net
Tom Atkinson is right. Both his case and mine are valid SQL. They can't be solved both the way it was and the way i proposed. Here is a new proposal to solve both: Every query which contains two times SELECT will be excecuted as is. current line 260: elseif (preg_match('#GROUP\s*BY#is', $this->_query) === 1 || preg_match('#\sUNION\s#is', $this->_query) === 1 || preg_match('#SELECT.*DISTINCT.*FROM#is', $this->_query) === 1 replace them with the following lines: elseif (preg_match('#GROUP\s*BY#is', $this->_query) === 1 || preg_match('#SELECT.*SELECT#is', $this->_query) === 1 || preg_match('#\sUNION\s#is', $this->_query) === 1 || preg_match('#SELECT.*DISTINCT.*FROM#is', $this->_query) === 1
 [2007-02-14 07:28 UTC] wiesemann (Mark Wiesemann)
Harry and Tom, thanks for your reports and patches. I'll do some more tests and will make a new release in the next days.
 [2007-03-11 09:35 UTC] wiesemann (Mark Wiesemann)
Thank you for your bug report. This issue has been fixed in the latest released version of the package, which you can download at http://pear.php.net/get/Structures_DataGrid_DataSource_MDB2