Doctrine 1
  1. Doctrine 1
  2. DC-421

Doctrine_Table::createQuery() can use wrong connection

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0, 1.2.1
    • Fix Version/s: None
    • 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. DC-421.patch
        0.8 kB
        Eugene Janusov
      2. DC421TestCase.php
        3 kB
        Eugene Janusov

        Activity

        Hide
        Eugene Janusov added a comment -

        Attached test case.

        Show
        Eugene Janusov added a comment - Attached test case.
        Hide
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: