ZF-5372: Wrong SQL generation in Zend_Db_Select::joinUsing()

Description

The SQL generated by Zend_Db_Select::joinUsing() when joining by an array of columns is totally broken.

Sample code to reproduce:


$db = Zend_Db::factory('Pdo_Mysql', array('dbname' => 'test', 'username' => 'root', 'password' => ''));
$select = new Zend_Db_Select($db);
$select->from('table_A')->joinUsing('tableB', array('col_one', 'col_two'));
// Outputs:
//  SELECT `table_A`.*, `tableB`.* FROM `table_A` INNER JOIN `tableB` ON `tableB`.Array = `table_A`.Array
echo (string) $select;

Notice the columns being used on the ON clause: ON tableB.{color:red}Array{color} = table_A.{color:red} Array{color}

This is a quick patch that solves the issue:


Index: Zend/Db/Select.php
===================================================================
--- Zend/Db/Select.php  (revision 13462)
+++ Zend/Db/Select.php  (working copy)
@@ -809,9 +809,17 @@
         $join  = $this->_adapter->quoteIdentifier(key($this->_parts[self::FROM]), true);
         $from  = $this->_adapter->quoteIdentifier($this->_uniqueCorrelation($name), true);
 
-        $cond1 = $from . '.' . $cond;
-        $cond2 = $join . '.' . $cond;
-        $cond  = $cond1 . ' = ' . $cond2;
+        if (is_scalar($cond)) {
+            $cond1 = $from . '.' . $cond;
+            $cond2 = $join . '.' . $cond;
+            $cond  = $cond1 . ' = ' . $cond2;
+        } else {
+            $conds = array();
+            foreach ($cond as $c) {
+                $conds[] = $from . '.' . $c . ' = ' . $join . '.' . $c;
+            }
+            $cond = implode(' AND ', $conds);
+        }
 
         return $this->_join($type, $name, $cond, $cols, $schema);
     }

Sample code output after applying above patch::


// Outputs:
//  SELECT `table_A`.*, `tableB`.* FROM `table_A` INNER JOIN `tableB` ON `tableB`.col_one = `table_A`.col_one AND `tableB`.col_two = `table_A`.col_two

It would be better if the generated SQL was using USING though.

Comments

It isn't a bug but an improvement because joinUsing wasn't designed to function with an array.

So may I ask what's the purpose of the joinUsing family of methods? I fail to see their usefulness.

Well then I would suggest to complete remove it, it serves no purpose: a) It does not generate a USING clause (see [http://framework.zend.com/issues/browse/ZF-3792]). b) It does not simplify code by mapping a column-list array into the proper AND'ed expressions (like what this issue is about).

I would then label this bug as a design bug, but a bug nevertheless.

I understand what you want, but this is the phpDoc associated to _joinUsing:


/**
 * Handle JOIN... USING... syntax
 *
 * This is functionality identical to the existing JOIN methods, however
 * the join condition can be passed as a single column name. This method
 * then completes the ON condition by using the same field for the FROM
 * table and the JOIN table.
 *
 * 
 * $select = $db->select()->from('table1')
 *                        ->joinUsing('table2', 'column1');
 *
 * // SELECT * FROM table1 JOIN table2 ON table1.column1 = table2.column2
 * 
 *
 * These joins are called by the developer simply by adding 'Using' to the
 * method name. E.g.
 * * joinUsing
 * * joinInnerUsing
 * * joinFullUsing
 * * joinRightUsing
 * * joinLeftUsing
 *
 * @return Zend_Db_Select This Zend_Db_Select object.
 */

Perhaps, joinUsing isn't a good name for this function ;).

Thanks for showing me the "fine print" ;)

How can I help to fix it? The patch in the issue (massaging parameter $cond) at least gives some utility to the method(s) IMO. Don't you agree?

After further checking, the implementation of joinUsing() is broken in another way too: table aliases are not being respected!


$select->from(array('a' => 'table_a'))
       ->joinUsing(array('b' => 'table_b'), 'col_X');

You would assume the generated SQL to be:


SELECT `a`.*, `b`.* FROM `table_a` AS `a` INNER JOIN `table_b` AS `b` ON `b`.col_X = `a`.col_X

But you would be wrong, the actual SQL generated is:


SELECT `a`.*, `b`.* FROM `table_a` AS `a` INNER JOIN `table_b` AS `b` ON `table_b`.col_X = `a`.col_X

Here is the less intrusive patch I could think of that solves this second problem:


Index: Zend/Db/Select.php
===================================================================
--- Zend/Db/Select.php  (revision 13471)
+++ Zend/Db/Select.php  (working copy)
@@ -826,6 +826,10 @@
     {
         if (is_array($name)) {
             $c = end($name);
+            // Respect table name aliases if present
+            if (!is_numeric(key($name))) {
+                $c = key($name);
+            }
         } else {
             // Extract just the last name of a qualified table name
             $dot = strrpos($name,'.');

The following join method in 1.7.5 still generates wrong SQL code

->joinUsing(array('b' => 'table_b'), 'col_X');

Any news here?

bq. Any news here?

Yes there are - fixed in trunk, see ZF-3309

@[~danielb] The table alias problem is being worked on in ZF-3309. The currently-agreed-upon solution is very similar to what you have proposed.

However, the original issue reported in this ticket has not been resolved. @[~ralph]: Is this something we want to add in ZFv1 at this stage? ie:


$select->from('table_A')->joinUsing('tableB', array('col_one', 'col_two'));
// Outputs:
//  SELECT `table_A`.*, `tableB`.* FROM `table_A` INNER JOIN `tableB` ON `tableB`.col_one = `table_A`.col_one AND `tableB`.col_two = `table_A`.col_two

This will be addressed in ZF2 Zend\Db