Fix merging related data in many_many relation.

[koyhoge]

I found bug with many_many relation. This P-R fix it.

Test environment

Create table

create table t1 (
  id integer primary key,
  v1 varchar(255)
);
create table t2 (
  id integer primary key,
  v2 varchar(255)
);
create table t1_t2_map (
  t1_id integer,
  t2_id integer
);
insert into t1 (id, v1) value (1, 'foo');
insert into t2 (id, v2) value (1, 'bar');
insert into t2 (id, v2) value (2, 'baz');
insert into t1_t2_map (t1_id, t2_id) value (1, 1);
insert into t1_t2_map (t1_id, t2_id) value (1, 2);

Inserted data

mysql> select * from t1;
+----+------+
| id | v1   |
+----+------+
|  1 | foo  |
+----+------+
1 row in set (0.00 sec)

mysql> select * from t2;
+----+------+
| id | v2   |
+----+------+
|  1 | bar  |
|  2 | baz  |
+----+------+
2 rows in set (0.00 sec)

mysql> select * from t1_t2_map;
+-------+-------+
| t1_id | t2_id |
+-------+-------+
|     1 |     1 |
|     1 |     2 |
+-------+-------+
2 rows in set (0.00 sec)

Model Classes

• Model_T1

class Model_T1 extends \Orm\Model
{
    /**
     * @var table name
     */
    protected static $_table_name = 't1';

    /**
     * relation definition
     */
    protected static $_many_many = [
        't2' => [
            'key_from' => 'id',
            'table_through' => 't1_t2_map',
            'key_through_from' => 't1_id',
            'key_through_to' => 't2_id',
            'model_to' => 'Model_T2',
            'key_to' => 'id',
            'cascade_save' => false,
            'cascade_delete' => false,
        ],
    ];
}

• Model_T2

class Model_T2 extends \Orm\Model
{
    /**
     * @var table name
     */
    protected static $_table_name = 't2';
}

Reproduce bug

        $result = \Model_T1::query()
            ->related('t2')
            ->get_one();
        var_dump($result->to_array());

Result:

array(3) {
  ["id"]=>
  int(1)
  ["v1"]=>
  string(3) "foo"
  ["t2"]=>
  array(1) {
    [1]=>
    array(2) {
      ["id"]=>
      int(1)
      ["v2"]=>
      string(3) "bar"
    }
  }
}

After this patch, correct result:

array(3) {
  ["id"]=>
  int(1)
  ["v1"]=>
  string(3) "foo"
  ["t2"]=>
  array(2) {
    [1]=>
    array(2) {
      ["id"]=>
      int(1)
      ["v2"]=>
      string(3) "bar"
    }
    [2]=>
    array(2) {
      ["id"]=>
      int(2)
      ["v2"]=>
      string(3) "baz"
    }
  }
}

[WanWizard]

What is the output of:

        $result = \Model_T1::query()
            ->related('t2')
            ->limit(1)->get();

        $result = reset($result);
        var_dump($result->to_array());

Is it identical or different? And what about

        $result = \Model_T1::query()
            ->related('t2')
            ->rows_limit(1)->get();

        $result = reset($result);
        var_dump($result->to_array());

[koyhoge]

OK, I'll check generated SQL:

            ->limit(1)->get();

Outputs:

SELECT
  `t0`.`id` AS `t0_c0`, 
  `t0`.`v1` AS `t0_c1`, 
  `t1_through`.`t2_id`,
  `t1_through`.`t1_id`,
  `t1`.`id` AS `t1_c0`,
  `t1`.`v2` AS `t1_c1`
  FROM (
    SELECT `t0`.`id`, `t0`.`v1` FROM `t1` AS `t0` LIMIT 1
 ) AS `t0`
  LEFT JOIN `t1_t2_map` AS `t1_through`
   ON (`t0`.`id` = `t1_through`.`t1_id`) 
 LEFT JOIN `t2` AS `t1`
  ON (`t1_through`.`t2_id` = `t1`.`id`)

and

            ->rows_limit(1)->get();

Outputs:

SELECT
   `t0`.`id` AS `t0_c0`,
   `t0`.`v1` AS `t0_c1`,
   `t1_through`.`t2_id`, 
  `t1_through`.`t1_id`, 
  `t1`.`id` AS `t1_c0`,
  `t1`.`v2` AS `t1_c1`
  FROM `t1` AS `t0`
  LEFT JOIN `t1_t2_map` AS `t1_through`
   ON (`t0`.`id` = `t1_through`.`t1_id`)
  LEFT JOIN `t2` AS `t1`
   ON (`t1_through`.`t2_id` = `t1`.`id`)
 LIMIT 1

So what are you saying? It returns only 1 related recoord with row_limit(1), but it depends internal design of the ORM. It's not about this issue.

It always returns a single related item with many_many relation on current code. Is this a correct behavoir?

[WanWizard]

rows_limit() should return a single row from the SQL, causing relations to be incomplete, limit() should always limit on the parent, and always return complete related record sets. So I just wanted to double-check.

@stevewest agree to merge this PR?

[emlynwest]

Looks ok to me.