Skip to content

Remove duplicate SQL parts in query generation #277

Closed
wants to merge 1 commit into from

4 participants

@gigablah
gigablah commented Mar 2, 2013

Presently, it's possible to do this using QueryBuilder:

$qb->select('u.*')
   ->from('users', 'u')
   ->addOrderBy('u.name', 'ASC')
   ->addOrderBy('u.name', 'ASC'); // duplicate

Which produces:

SELECT u.* FROM users u ORDER BY u.name ASC, u.name ASC

This patch removes duplicates from the SELECT, SET, ORDER BY and GROUP BY parts during SQL generation (there's already logic that removes duplicate FROM parts).

I've added test cases for each affected keyword as well.

@doctrinebot

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-454

@mvrhov
mvrhov commented Mar 2, 2013

I've made a similar PR which removed duplicate joins and it was turned down with an explanation that I should track this in my own code. Go figure...

@Ocramius
Doctrine member
Ocramius commented Mar 2, 2013

Correct. It should actually be up to you to produce valid SQL. The QueryBuilder is only a string builder.

If you do mistakes, doctrine is not here to fix them for you :)

@gigablah
gigablah commented Mar 2, 2013

Sure.

In the meantime, I'll just use this:

foreach (array('select', 'set', 'groupBy', 'orderBy') as $keyword) {
    $qb->add($keyword, array_unique($qb->getQueryPart($keyword)));
}

As a bonus, here's one for the ORM QueryBuilder, which is a little bit more involved:

foreach (array('select', 'set', 'groupBy', 'orderBy') as $keyword) {
    $parts = array_unique($qb->getDQLPart($keyword));
    $qb->resetDQLPart($keyword);
    foreach ($parts as $part) {
        $qb->add($keyword, $part, true);
    }
}

I guess we could create another open source library to sanitize QueryBuilder output :)

@mvrhov
mvrhov commented Mar 2, 2013

The reasoning behind my case was that I have to build the query based on the parameters I get from external source via API. And sometimes the joins could span about 5-6 tables in various combinations. As I have to track the joins myself the code contains numerous if's that could have been prevented if query builder were a bit smarter.

@gigablah
gigablah commented Mar 3, 2013

Yeah, that's my use case too, generating queries from parameters. I guess I'll check out your solution for joins.

@gigablah gigablah closed this Mar 3, 2013
@gigablah gigablah deleted the unknown repository branch Mar 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.