Doctrine 1

Doctrine_Table::createQuery() can use wrong connection

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.2.0, 1.2.1
  • Fix Version/s: 1.2.4
  • Component/s: Connection, Query
  • Labels:
    None

Description

r6799 introduced 3 changes to the Doctrine_Table: in findByDql(), getQueryObject(), and createQuery() methods.
The first two are pretty obvious, but the last one is a mistake in my opinion:

--- lib/Doctrine/Table.php	(revision 6798)
+++ lib/Doctrine/Table.php	(revision 6799)
@@ -1033,7 +1033,7 @@
 
         $class = $this->getAttribute(Doctrine_Core::ATTR_QUERY_CLASS);
 
-        return Doctrine_Query::create($this->_conn, $class)
+        return Doctrine_Query::create(null, $class)
             ->from($this->getComponentName() . $alias);
     }

An instance of Doctrine_Table always has a reference to a particular Doctrine_Connection, so why createQuery() method, intended to "create a query on this table" should use different (i.e. default one) connection instead of already specified one?

  1. Text File
    DC-421.patch
    20/Jan/10 1:04 AM
    0.8 kB
    Eugene Janusov
  2. File
    DC421TestCase.php
    20/Jan/10 1:22 AM
    3 kB
    Eugene Janusov

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Eugene Janusov added a comment -

Attached test case.

Show
Eugene Janusov added a comment - Attached test case.
Hide
Permalink
Eugene Janusov added a comment - - edited

While preparing the test case, I looked more closely to the sources. It turns out that if you do something like this:

Doctrine::getTable('ComponentName')->createQuery()->execute()

then eventually the preferred connection will be used.

But createQuery() method creates a new Doctrine_Query and doesn't pass a connection, thus Doctrine_Query_Abstract's constructor tries to get a default connection, which is unacceptable if you'd like to work without a default connection.

A possible solution that I see is to not set $this>_conn inside Doctrine_Query as long as it will not be really required.-

Show
Eugene Janusov added a comment - - edited While preparing the test case, I looked more closely to the sources. It turns out that if you do something like this: Doctrine::getTable('ComponentName')->createQuery()->execute() then eventually the preferred connection will be used. But createQuery() method creates a new Doctrine_Query and doesn't pass a connection, thus Doctrine_Query_Abstract 's constructor tries to get a default connection, which is unacceptable if you'd like to work without a default connection. A possible solution that I see is to not set $this >_conn inside Doctrine_Query as long as it will not be really required.-
Hide
Permalink
Eugene Janusov added a comment - - edited

I found another solution. As I said above, the problem is really relevant only if you don't use a default connection. But in this case you must bind all your components to a particular connection(s). So, we can just check for hasConnectionForComponent() inside Doctrine_Table::createQuery().

Show
Eugene Janusov added a comment - - edited I found another solution. As I said above, the problem is really relevant only if you don't use a default connection. But in this case you must bind all your components to a particular connection(s). So, we can just check for hasConnectionForComponent() inside Doctrine_Table::createQuery() .
Hide
Permalink
Jonathan H. Wage added a comment -

Can you provide the fix you found as a patch so I can clearly see it and test it? Thanks, Jon

Show
Jonathan H. Wage added a comment - Can you provide the fix you found as a patch so I can clearly see it and test it? Thanks, Jon
Hide
Permalink
Paul Kamer added a comment -

Jonathan, I can confirm that this patch solves this issue.
It also fixes this ticked in Symfony 1.X: http://trac.symfony-project.org/ticket/7689

Show
Paul Kamer added a comment - Jonathan, I can confirm that this patch solves this issue. It also fixes this ticked in Symfony 1.X: http://trac.symfony-project.org/ticket/7689
Hide
Permalink
Jochen Bayer added a comment -

DC-421.patch works for me too.

Show
Jochen Bayer added a comment - DC-421 .patch works for me too.
Hide
Permalink
Jonathan H. Wage added a comment -

I've reverted that one change and everything seems better now: https://github.com/doctrine/doctrine1/commit/b4dc8e66a89a7e17cd195c489b18005e19ca9ea5

Show
Jonathan H. Wage added a comment - I've reverted that one change and everything seems better now: https://github.com/doctrine/doctrine1/commit/b4dc8e66a89a7e17cd195c489b18005e19ca9ea5

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    Eugene Janusov
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: