zendframework/zend-db

Bug: select include all columns from joined tables (2.8.0 breaks)

Closed this issue · 3 comments

After upgrade from 2.7.0 to 2.8.0 :

When joining tables in a Select object, the buildSqlString() method includes all columns from joined tables (SQL_STAR) although they should be discarded by the third param in the Select::join($name, $on, $columns = self::SQL_STAR, $type = self::JOIN_INNER) method.

This bug may lead to really poor performance after upgrade as joined tables are often used to filter results or even worse disclosing information in case the query is used to generate data (api). This probably may be marked as security bugfix as well.

This bug have been tested with php 5.6.20 / 7, Mysql, Oracle, Sql92 platforms.

Take a look at this example
(notice the $columns=[] param in the join() method and see also the columns()):

$select = new \Zend\Db\Sql\Select();
$select->from('my_table')
       ->join('joined_table', 'my_table.id = joined_table.id', $columns=[])
       ->columns([
          'my_table_column',
       ]);
$sql_string = $this->sql->buildSqlString($select, $this->getAdapterForPlatform('Sql92'));

On 2.7.0 and before the resulting query string is correctly only producing one column:

SELECT 
    "my_table"."my_table_column" AS "my_table_column", 
FROM "my_table" 
INNER JOIN "joined_table" ON "my_table"."id" = "joined_table"."id"

After update to 2.8.0, the resulting query string incorrectly add all columns from the joined table. Take a look on the third line (joined_table.*)

SELECT 
    "my_table"."my_table_column" AS "my_table_column", 
    "joined_table".*
FROM "my_table" 
INNER JOIN "joined_table" ON "my_table"."id" = "joined_table"."id"

I'm currently looking to provide a P/R and add some specs into the SqlFunctionnalTests. Feel free to help.

Pull request and possible fix on its way, see #100

weckx commented

I can confirm this bug and it also breaks existing code, for example the query below that has two joins:

$select = $sql->select('news');
$select->join('company',` 'company.id = news.company_id');
$select->join('category', 'category.id = news.category_id');

If all the tables have an "id" field, In MySQL this will cause a "Duplicate column name", thus throwing an exception.

For my case @belgattitude PR fixes the problem.

Hi Felipe,

Good to know...

I'm a bit curious, looking at your code snippet I feel my fix is not supposed to fix it ;) For that your code should explicitly pass an empty array as 3rd param like :

$select = $sql->select('news');
$select->join('company',` 'company.id = news.company_id', []);
$select->join('category', 'category.id = news.category_id', []);
$select->columns([.....]);

Or am I missing something ? Just asking in case I forgot some use cases in the tests

Anyway I've just updated the PR with more tests and optimization.

Can you test with the latest ?

Thanks,

Seb