Doctrine 1
  1. Doctrine 1
  2. DC-651

[PATCH] Doctrine_Record::option('orderBy', ...) of join's right side being applied to refTable in m2m relationship

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.2.2, 1.2.3
    • Fix Version/s: 1.2.2, 1.2.3
    • Component/s: Query, Relations
    • Labels:
      None
    • Environment:
      CentOS 5.4
      PHP 5.3.2
      MySQL 5.1.44, for unknown-linux-gnu (x86_64)

      Description

      When using the Doctrine_Record::option('orderBy', ...) feature on a table definition, where that table is the target of a many-to-many join, the specified orderBy columns are applied to the relation table's alias. So for example, given the following definitions:

      class User extends Doctrine_Record {
        public function setTableDefinition() {
          $this->hasColumn('uid', 'integer', null, array('primary' => true));
          $this->option('orderBy', 'uid');
        }
      
        public function setUp() {
          $this->hasMany('Group as groups', array('refClass' => 'UserGroup', 'local' => 'user_uid', 'foreign' => 'group_id'));
        }
      }
      
      class Group extends Doctrine_Record {
        public function setTableDefinition() {
          $this->hasColumn('gid', 'integer', null, array('primary' => true));
        }
      
        public function setUp() {
          $this->hasMany('User as users', array('refClass' => 'UserGroup', 'local' => 'group_gid', 'foreign' => 'user_id'));
        }
      }
      
      class UserGroup extends Doctrine_Record {
        public function setTableDefinition() {
          $this->hasColumn('user_uid', 'integer', null, array('primary' => true));
          $this->hasColumn('group_gid', 'integer', null, array('primary' => true));
        }
      
        public function setUp() {
          $this->hasOne('User as user', array('local' => 'user_uid', 'foreign' => 'uid'));
          $this->hasOne('Group as group', array('local' => 'group_gid', 'foreign' => 'gid'));
        }
      }
      

      the following queries:

      $query = Doctrine_Query::create()
        ->select('u.*')
        ->from('User u')
        ->leftJoin('u.groups g WITH g.gid=?', 1);
      echo $query->getSqlQuery() . "\n";
      
      $query = Doctrine_Query::create()
        ->select('g.*')
        ->from('Group g')
        ->leftJoin('g.users u WITH u.uid=?', 1);
      echo $query->getSqlQuery() . "\n";
      

      will output the following:

      SELECT u.uid AS u__uid FROM user u LEFT JOIN user_group u2 ON (u.uid = u2.user_uid) LEFT JOIN group g ON g.gid = u2.group_id AND (g.gid = ?) ORDER BY u.uid
      SELECT g.gid AS g__gid FROM group g LEFT JOIN user_group u2 ON (g.gid = u2.group_gid) LEFT JOIN user u ON u.uid = u2.user_id AND (u.uid = ?) ORDER BY u.uid, u2.uid

      The orderBy option() call is applied to the User definition. The SQL for the first query is correct (where User is on the left side of the join). The SQL for the second query (where User is on the right-most side of the join), however, is obviously incorrect (UserGroup doesn't even have a uid column). Basically, User's orderBy option is being applied to both the User table and its respective reference table, UserGroup, when it is the target of a join.

      After digging through the source for a while, I believe I've come up with a patch for this issue (which should be checked by someone more knowledgeable of Doctrine's internals). Basically, in the Doctrine_Query::buildSqlQuery() function, a call is made to Doctrine_Relation::getOrderByStatement() with the reference table (UserGroup)'s alias (u2), which in turn makes a call to Doctrine_Table::getOrderByStatement() on the referenced table (User), filling in the ORDER BY clause with User columns using UserGroup's alias. My solution was to reorder the logic so that the test for a reference class is made before the initial call to getOrderByStatement() is made. It seems to work against my test case and the test cases in the repository. I'll post my patch momentarily.

      This bug was first mentioned in the comments in DC-313, but the original ticket comes across as more of a feature request for the hasMany() orderBy feature.

      1. DC651TestCase.php
        3 kB
        suhock
      2. Query_orderBy_relation.diff
        1 kB
        Dan Ordille
      3. Ticket_DC651.patch
        1 kB
        suhock

        Activity

        Hide
        suhock added a comment -

        attached a test case for this bug

        Show
        suhock added a comment - attached a test case for this bug
        Hide
        suhock added a comment -

        patch against /branches/1.2 HEAD (should also work apply to 1.2.2 tag)

        Show
        suhock added a comment - patch against /branches/1.2 HEAD (should also work apply to 1.2.2 tag)
        Hide
        Dan Ordille added a comment -

        I can confirm this as an issue. However I don't think the above patch adequately fixes the problem it seems like with it an order by is still added for the ref column however the relation alias is lost.

        My query with the patch became
        SELECT g.gid AS g__gid FROM group g LEFT JOIN user_group u2 ON (g.gid = u2.group_gid) LEFT JOIN user u ON u.uid = u2.user_id AND (u.uid = ?) ORDER BY u.uid, uid

        I made an another patch that prevents this extra order by clause from being added and have attached it.

        Show
        Dan Ordille added a comment - I can confirm this as an issue. However I don't think the above patch adequately fixes the problem it seems like with it an order by is still added for the ref column however the relation alias is lost. My query with the patch became SELECT g.gid AS g__gid FROM group g LEFT JOIN user_group u2 ON (g.gid = u2.group_gid) LEFT JOIN user u ON u.uid = u2.user_id AND (u.uid = ?) ORDER BY u.uid, uid I made an another patch that prevents this extra order by clause from being added and have attached it.
        Hide
        suhock added a comment -

        I tried out the new patch (Query_orderby_relation.diff), but it provides a reversed diff (patching goes from a patched version to the original). After applying it manually, it fails the provided test case and several additional test cases from the repository.

        The original patch DOES pass the provided test case, when applied against 1.2.2, 1.2.3, or the 1.2 branch from the repository. It does not pass, however, Doctrine_Query_Orderby_TestCase. As the previous poster mentioned, it fails to resolve aliases in instances where the 'orderBy' option is specified in a relation definition.

        I deleted the original patch and am providing a revised patch (Ticket_DC651.patch) against branch 1.2 HEAD (also works with 1.2.3), which fixes this issue. It passes all working test cases, including Doctrine_Query_Orderby_TestCase and DC651TestCase.

        Show
        suhock added a comment - I tried out the new patch (Query_orderby_relation.diff), but it provides a reversed diff (patching goes from a patched version to the original). After applying it manually, it fails the provided test case and several additional test cases from the repository. The original patch DOES pass the provided test case, when applied against 1.2.2, 1.2.3, or the 1.2 branch from the repository. It does not pass, however, Doctrine_Query_Orderby_TestCase. As the previous poster mentioned, it fails to resolve aliases in instances where the 'orderBy' option is specified in a relation definition. I deleted the original patch and am providing a revised patch (Ticket_DC651.patch) against branch 1.2 HEAD (also works with 1.2.3), which fixes this issue. It passes all working test cases, including Doctrine_Query_Orderby_TestCase and DC651TestCase.
        Hide
        José De Araujo added a comment -

        I had this issue recently on a application I'm working on as described the oderBy option was applied on the joined table on a column that even doesn't exist in it. I used the DC651 patch provided and it solved the issue, so far I haven't seen any side effect to it.

        Show
        José De Araujo added a comment - I had this issue recently on a application I'm working on as described the oderBy option was applied on the joined table on a column that even doesn't exist in it. I used the DC651 patch provided and it solved the issue, so far I haven't seen any side effect to it.

          People

          • Assignee:
            Guilherme Blanco
            Reporter:
            suhock
          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: