Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Paginator\Adapter\DbSelect additional for mistake elimination #4673

Closed
wants to merge 4 commits into from
Closed

Conversation

isimonov
Copy link

This is a fix for issue #4641

Eliminates an error: "PDOException: SQLSTATE[42S21]: Column already exists: 1060 Duplicate column..." when original_select contains columns called in selection by the same name ( e.g. TRIM(str_column) as str_column ).

In issue did not reset columns in select query to determine the count of rows in the grouped queries. Here there was a problem in my opinion. The count() method throws an error when original_select will be such:

SELECT *, TRIM(str_column) as str_column FROM table

PDOException: SQLSTATE[42S21]: Column already exists: 1060 Duplicate column name 'str_column'

Query is good work: SELECT *, TRIM(str_column) as str_column FROM table

But query determine the count of rows throws an error:
SELECT COUNT(1) AS "c" FROM ( SELECT *, TRIM(str_column) as str_column FROM table ) AS "original_select"

The error resulted in higher.

I think need to reset columns original_select on '*'.

$select->reset(Select::COLUMNS);
$select->columns(array(Select::SQL_STAR));

So everything will work correctly.

If in join uses the same technique ( e.g. TRIM(str_column) as str_column ) appears the same error. For this need to reset the column in join. Since it was originally.

Eliminates an error: "PDOException: SQLSTATE[42S21]: Column already exists: 1060 Duplicate column..." when original_select contains columns called in selection by the same name ( e.g. TRIM(str_column) as str_column ).
@weierophinney
Copy link
Member

Unit tests, please.

@ThaDafinser
Copy link
Contributor

@Latch do you still work on this?

@weierophinney
Copy link
Member

@Latch There's a test failure -- DbSelectTest::testCount fails.

@ewertonvaz
Copy link

I had the same problem when using DbSelect->count() in a joined table, so I created a class named MyDbSelect as follow:

class MyDbSelect extends DbSelect
{
public function count()
{
if ($this->rowCount !== null) {
return $this->rowCount;
}

    $select = clone $this->select;
    $select->reset(Select::LIMIT);
    $select->reset(Select::OFFSET);
    $select->reset(Select::ORDER);
    $select->reset(Select::COLUMNS); // <-- Include this to work in Joined table

    $countSelect = new Select;
    $countSelect->columns(array('c' => new Expression('COUNT(1)')));
    $countSelect->from(array('original_select' => $select));
    //you can check your query by echo-ing :
    //echo $countSelect->getSqlString();

    $statement = $this->sql->prepareStatementForSqlObject($countSelect);
    $result    = $statement->execute();
    $row       = $result->current();

    $this->rowCount = $row['c'];

    return $this->rowCount;
}

}

@ralphschindler
Copy link
Member

Is this a valid SELECT?

SELECT *, TRIM(str_column) as str_column FROM table

on what platform, mysql?

@ewertonvaz
Copy link

Yes it is valid (in MYSQL) a new column will be created with data without prefixes and / or suffixes, Read about in:
http://www.w3resource.com/mysql/string-functions/mysql-trim-function.php

@ralphschindler
Copy link
Member

I'm not asking about the trim(), I'm asking about the portion that does *, column_name_that_is_also_covered_by_* part. I'll test it. To me, that reads that there will be a duplicate column in the result.

@froschdesign
Copy link
Member

To me, that reads that there will be a duplicate column in the result.

Right.

@ralphschindler
Copy link
Member

I just don't think we can reset the columns carte blanche like that. Think of situations where someone has utilized the DISTINCT modifier. Ideally, a query would not produce duplicate named columns (btw, so far in my test, this only appears to affect MySQL, still going to test SQL Server, Db2 and Oracle yet).

What do you think about adding an option to the constructor to allow resetting of the columns to make it opt-in?

@ralphschindler
Copy link
Member

Closing due to inactivity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants