Ticket #1133 (closed defect: fixed)

Opened 13 months ago

Last modified 12 months ago

Count query with a join condition can put its parameters in the wrong order

Reported by: dball Owned by: jwage
Priority: major Milestone: 0.11.0
Component: Attributes Version: 0.11.0
Severity: Keywords:
Cc: Has Test: no
Status: Pending User Response Has Patch: no

Description

If you create a query with a join with a parameter, and a where clause with a parameter, and issue a count() query on it, the parameters are populated in the wrong order. I don't have time for a proper test case now, but the basic gist is:

$query->from('Foo f')->innerJoin('f.Bar b ON b.id = ?', $bar)->addWhere('f.id = ?', $foo);

$query->count(); // Uses foo for bar and vice versa

$query->execute(); // Uses the parameters in the proper places

Change History

Changed 13 months ago by jwage

  • status changed from new to closed
  • resolution set to fixed

(In [4519]) fixes #1133

Changed 13 months ago by jwage

  • mystatus changed from Pending Core Response to Commit Approved
  • milestone changed from 0.11.3 to 0.11.0

Changed 13 months ago by Vladev

  • status changed from closed to reopened
  • mystatus changed from Commit Approved to Pending Core Response
  • resolution fixed deleted

Since I updated my svn external, things started to blow up.

Here's some code (:

public function testDoctrineCountQuery()
{
      $query = Doctrine_Query::create()
           ->from('App_Api_Offer o')
           ->leftJoin('o.Images i')
           ->where('o.id = ?', -1)
           ->limit(1)
           ->offset(1);
       $this->assertEquals(0, $query->count());
}

The relations are fine. If I write this assertion, instead of the one above

$this->assertEquals(0, $query->execute()->count());

the test passes. If I remove the limit/offset stuff, it, yet again, passes.

I managed to track the change to r4519, which is marked as a fix for this bug.

Here is the error I get a the stack trace:

1) testDoctrineCountQuery(Carutoo_Api_OfferTest)
Doctrine_Connection_Mysql_Exception: SQLSTATE[42000]: 
Syntax error or access violation: 1064 
You have an error in your SQL syntax; check the manual that corresponds
to your MySQL server version for the right syntax to 
use near '? LIMIT 1 OFFSET 1' at line 1

C:workspacecarutoolibDoctrineDoctrineConnection.php:1004
C:workspacecarutoolibDoctrineDoctrineQuery.php:1164
C:workspacecarutoolibDoctrineDoctrineQueryAbstract.php:1974
C:workspacecarutoolibDoctrineDoctrineQuery.php:1758
C:workspacecarutoolibDoctrineDoctrineQuery.php:1831
C:workspacecarutoo	estsCarutooApiOfferTest.php:94
C:workspacecarutoolibendLoader.php:83
C:workspacecarutoolibendLoader.php:181
C:workspacecarutoo	ests
un.php:12

and the query (from the mysql log file)

SELECT DISTINCT o2.id FROM offers o2 LEFT JOIN images i2 ON o2.id = i2.offerId WHERE o2.id = ? LIMIT 1 OFFSET 1

Changed 13 months ago by guilhermeblanco

  • mystatus changed from Pending Core Response to Pending User Response

Vladev, I need a couple more information about this issue.

The reason I'm asking this to you is because I found a deeper bug and we are investigating, but your reopened made me find it. Actually, just as a quick overview, limit-subquery is never applied in count(), so it made me curious about how it may worked for you before.

First of all, did the test was untouched, you svn up and it broke from one day to other or did you change something? Try to remember if you added limit/offset or anything else.

Second, have you tried to revert to changeset prior to r4519 and check if the test pass without that commit?

Also, what's the output of getCountQuery() *LOOK* This method didn't exist before r4519. So the only change that happened is the params order.

Try to remove the params from methods and place as the argument of count.

Why am I asking these questions? Ok, let me try to be explicit. The actual query that runs in count() is very different from the one of execute(). It's something like SELECT COUNT(DISTINCT u.id) as num_results FROM users u ... GROUP BY u.id And join is the first thing that is applied to the query. So the only change we made is actually to move join as the first index of merging. Your DQL has nothing related to it, so I can't figure it out how it started to blow up.

After my research, I found these alternatives: 1- You are reusing the query. This can lead to a *POSSIBLE* bad cleanup of params 2- You were using execute()->count() and changed to count(). Then we found the issue 3- You scrambled the params from argument to be merged in source. This can lead to another misterious bug 4- You applied limit/offset and then it stopped to work. This made us to found the issue 5- You haven't touched the code. NO idea what's happening then.... =

Changed 13 months ago by Vladev

  • mystatus changed from Pending User Response to Pending Core Response

Ok, on Monday I will be able to give you more information, but I will answer some of the questions now so you have more time to think on the issue.

The test that is in the comment did not existed before I found the issue - I wrote it particularly to report here.

Everything was working when I wrote it, but yesterday I made and svn up and it blew up. I tried reverting prior to r4519 (actually did that after posting here) but, for totally unknown reason, that didn't helped :) Tried moving the parameters in count() - that didn't fix it, either. (will try again though, might have missed something) Haven't used execute()->count(). First do ->count() then execute(). The query is not reused (used once per request).

On Monday I will try to dig deeper and give you more information.

Thanks for Doctrine, btw :)

Changed 13 months ago by Vladev

Okay, so here's more information (:

public function testDoctrineCountQuery()
{
      $query = Doctrine_Query::create()
             ->from('Carutoo_Api_Offer o')
             ->leftJoin('o.Images i')
             ->where('o.id = ?', -1)
             ->limit(1)
             ->offset(1);
                   
      echo $query->getCountQuery();
}

The above does not execute at all. It throws the same error as in my first comment (MYSQL syntax error...). Doesn't even get to the echo line.

The strange thing is that this executes and passes, but I thing it's not running the right queries:

public function testDoctrineCountQuery()
{
      $query = Doctrine_Query::create()
           ->from('Carutoo_Api_Offer o')
           ->leftJoin('o.Images i')
           ->where('o.id = ?')
           ->limit('?')
           ->offset('?');
           
       echo $query->getCountQuery();
           
       $this->assertEquals(0, $query->count(array (-1)));
       $this->assertEquals(0, $query->execute(array (-1))->count());
}

Here, the output of the getCountQuery() is

SELECT COUNT(DISTINCT o.id) AS num_results FROM offers o 
LEFT JOIN images i ON o.id = i.offerId WHERE o.id = ? GROUP BY o.id

and that is valid.

I can't find a way to set any value for the limit and offset placeholders.

Hope that will help you resolve this issue :)

Changed 12 months ago by jwage

Can we create a separate ticket for this? After reading through it sounds like this is another issue with the limit subquery algorithm not being applied to count() queries when it is necessary to do so. Can one of you confirm this is true or false?

Changed 12 months ago by guilhermeblanco

  • mystatus changed from Pending Core Response to Pending User Response

I can confirm it and I also have a partial patch to it. My knowledge in limit-subquery algorithm is my blocker to complete it.

Vladev, could you please create another ticket for this?

Changed 12 months ago by Vladev

Sure, here is is: #1174 :)

Changed 12 months ago by jwage

  • status changed from reopened to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.