Issues

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

Issue Type: Improvement Created: 2008-12-29T05:34:27.000+0000 Last Updated: 2012-05-05T02:36:40.000+0000 Status: Closed Fix version(s): - Next Major Release ()

Reporter: Daniel Berstein (danielb) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_Db_Select

  • zf-caretaker-adamlundrigan
  • zf-crteam-needsexpert
  • zf-crteam-priority
  • zf-crteam-review

Related issues: - ZF-3792

Attachments:

Description

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

Sample code to reproduce:

<pre class="highlight">
$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:

<pre class="highlight">
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::

<pre class="highlight">
// 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

Posted by Mickael Perraud (mikaelkael) on 2008-12-29T05:41:26.000+0000

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

Posted by Daniel Berstein (danielb) on 2008-12-29T06:43:43.000+0000

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.

Posted by Mickael Perraud (mikaelkael) on 2008-12-29T07:08:28.000+0000

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

<pre class="highlight">
/**
 * 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 ;).

Posted by Daniel Berstein (danielb) on 2008-12-29T10:07:55.000+0000

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!

<pre class="highlight">
$select->from(array('a' => 'table_a'))
       ->joinUsing(array('b' => 'table_b'), 'col_X');

You would assume the generated SQL to be:

<pre class="highlight">
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:

<pre class="highlight">
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:

<pre class="highlight">
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,'.');

Posted by Niels Lensink (nielslensink) on 2009-03-10T02:23:20.000+0000

The following join method in 1.7.5 still generates wrong SQL code

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

Posted by Sonntag (sunday) on 2010-07-09T08:09:25.000+0000

Any news here?

Posted by Kim Blomqvist (kblomqvist) on 2011-08-20T17:46:14.000+0000

bq. Any news here?

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

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-17T01:28:06.000+0000

@[~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:

<pre class="highlight">
$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

Posted by Adam Lundrigan (adamlundrigan) on 2012-05-05T02:36:40.000+0000

This will be addressed in ZF2 Zend\Db

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts