Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix for DBAL-172 #175

Merged
merged 2 commits into from

7 participants

@wimvds

Apparently a real fix has never been included, don't know why exactly. But there are use cases where you want to join multiple tables without selecting any data from the joined tables.

Ie. a query like this one :
SELECT COUNT(DISTINCT news.id) FROM cb_newspages news
INNER JOIN nodeversion nv ON nv.refId = news.id AND nv.refEntityname='News'
INNER JOIN nodetranslation nt ON nv.nodetranslation = nt.id
INNER JOIN node n ON nt.node = n.id
WHERE nt.lang = 'nl' AND n.deleted != 1

could be written with the querybuilder (using this patch) as follows :

    $querybuilder->select('COUNT(DISTINCT news.id)')
        ->from('cb_newspages', 'news')
        ->innerJoin('news', 'nodeversion', 'nv', 'nv.refId = news.id AND nv.refEntityname=\'Entity\\\\News\'')
        ->innerJoin('nv', 'nodetranslation', 'nt', 'nv.nodetranslation = nt.id')
        ->innerJoin('nt', 'node', 'n', 'nt.node = n.id')
        ->where('nt.lang = :lang AND n.deleted != 1')
        ->setParameter('lang', $locale);

When you use an alias that isn't chained or used in a from clause it will still trigger a QueryException (as before).
ie.

    $querybuilder->select('COUNT(DISTINCT news.id)')
        ->from('cb_newspages', 'news')
        ->innerJoin('news', 'nodeversion', 'nv', 'nv.refId = news.id AND nv.refEntityname=\'Entity\\\\News\'')
        ->innerJoin('invalid', 'nodetranslation', 'nt', 'nv.nodetranslation = nt.id')
        ->innerJoin('nt', 'node', 'n', 'nt.node = n.id')
        ->where('nt.lang = :lang AND n.deleted != 1')
        ->setParameter('lang', $locale);

Will trigger the following QueryException :

"The given alias 'invalid' is not part of any FROM or JOIN clause table. The currently registered aliases are: news, nv, nt, n."

@travisbot

This pull request passes (merged 70cb967 into b1562a5).

@wimvds

Could someone please provide some feedback? As it is, the DBAL QueryBuilder is just way to limited for real-world use, you should have the ability to join tables without selecting info from them (with the current implementation this is impossible, this PR fixes that issue).

@stof

Please add a test for the new supprted case to avoid regressions

@wimvds

We're using this in production for 2 months now, and encountered no issues. Since I think this functionality would be quite beneficial for others as well (because the current limitations of the QueryBuilder make it unfit for real-world use imho), I'm wondering what else I can do to get this into master...

@youknowriad

Is this going to be merged ? We're blocked because of this. Thanks

@wimvds

Using it in production for 4 months now... Is there any chance this PR will ever be merged?

@beberlei beberlei merged commit 6ace93c into from
@beberlei
Owner

Sorry, didnt see this PR before for whatever reason, its totally valid and merged now :)

@wimvds

Thanks! This makes my day :p.

@dol

:boom: Thx

@yethee

This changes break the query building, when uses multiple from parts with joins.
http://www.doctrine-project.org/jira/browse/DBAL-442

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 18, 2012
  1. @wimvds

    Fix for DBAL-172

    wimvds authored
Commits on Sep 12, 2012
  1. @wimvds

    regression test for DBAL-172

    wimvds authored
This page is out of date. Refresh to see the latest.
View
37 lib/Doctrine/DBAL/Query/QueryBuilder.php
@@ -946,29 +946,36 @@ private function getSQLForSelect()
$query = 'SELECT ' . implode(', ', $this->sqlParts['select']) . ' FROM ';
$fromClauses = array();
-
+ $joinsPending = true;
+ $joinAliases = array();
+
// Loop through all FROM clauses
foreach ($this->sqlParts['from'] as $from) {
$fromClause = $from['table'] . ' ' . $from['alias'];
- if (isset($this->sqlParts['join'][$from['alias']])) {
- foreach ($this->sqlParts['join'][$from['alias']] as $join) {
- $fromClause .= ' ' . strtoupper($join['joinType'])
- . ' JOIN ' . $join['joinTable'] . ' ' . $join['joinAlias']
- . ' ON ' . ((string) $join['joinCondition']);
- }
- }
-
+ if ($joinsPending && isset($this->sqlParts['join'][$from['alias']])) {
+ foreach ($this->sqlParts['join'] as $joins) {
+ foreach ($joins as $join) {
+ $fromClause .= ' ' . strtoupper($join['joinType'])
+ . ' JOIN ' . $join['joinTable'] . ' ' . $join['joinAlias']
+ . ' ON ' . ((string) $join['joinCondition']);
+ $joinAliases[$join['joinAlias']] = true;
+ }
+ }
+ $joinsPending = false;
+ }
+
$fromClauses[$from['alias']] = $fromClause;
}
// loop through all JOIN clauses for validation purpose
- foreach ($this->sqlParts['join'] as $fromAlias => $joins) {
- if ( ! isset($fromClauses[$fromAlias]) ) {
- throw QueryException::unknownFromAlias($fromAlias, array_keys($fromClauses));
- }
- }
-
+ $knownAliases = array_merge($fromClauses,$joinAliases);
+ foreach ($this->sqlParts['join'] as $fromAlias => $joins) {
+ if ( ! isset($knownAliases[$fromAlias]) ) {
+ throw QueryException::unknownAlias($fromAlias, array_keys($knownAliases));
+ }
+ }
+
$query .= implode(', ', $fromClauses)
. ($this->sqlParts['where'] !== null ? ' WHERE ' . ((string) $this->sqlParts['where']) : '')
. ($this->sqlParts['groupBy'] ? ' GROUP BY ' . implode(', ', $this->sqlParts['groupBy']) : '')
View
8 lib/Doctrine/DBAL/Query/QueryException.php
@@ -29,12 +29,10 @@
*/
class QueryException extends DBALException
{
- static public function unknownFromAlias($alias, $registeredAliases)
+ static public function unknownAlias($alias, $registeredAliases)
{
return new self("The given alias '" . $alias . "' is not part of " .
- "any FROM clause table. The currently registered FROM-clause " .
- "aliases are: " . implode(", ", $registeredAliases) . ". Join clauses " .
- "are bound to from clauses to provide support for mixing of multiple " .
- "from and join clauses.");
+ "any FROM or JOIN clause table. The currently registered " .
+ "aliases are: " . implode(", ", $registeredAliases) . ".");
}
}
View
37 tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php
@@ -556,17 +556,32 @@ public function testReferenceJoinFromJoin()
{
$qb = new QueryBuilder($this->conn);
- $qb->select("l.id", "mdsh.xcode", "mdso.xcode")
- ->from("location_tree", "l")
- ->join("l", "location_tree_pos", "p", "l.id = p.tree_id")
- ->rightJoin("l", "hotel", "h", "h.location_id = l.id")
- ->leftJoin("l", "offer_location", "ol", "l.id=ol.location_id")
- ->leftJoin("ol", "mds_offer", "mdso", "ol.offer_id = mdso.offer_id")
- ->leftJoin("h", "mds_hotel", "mdsh", "h.id = mdsh.hotel_id")
- ->where("p.parent_id IN (:ids)")
- ->andWhere("(mdso.xcode IS NOT NULL OR mdsh.xcode IS NOT NULL)");
-
- $this->setExpectedException('Doctrine\DBAL\Query\QueryException', "The given alias 'ol' is not part of any FROM clause table. The currently registered FROM-clause aliases are: l");
+ $qb->select('COUNT(DISTINCT news.id)')
+ ->from('cb_newspages', 'news')
+ ->innerJoin('news', 'nodeversion', 'nv', 'nv.refId = news.id AND nv.refEntityname=\'News\'')
+ ->innerJoin('invalid', 'nodetranslation', 'nt', 'nv.nodetranslation = nt.id')
+ ->innerJoin('nt', 'node', 'n', 'nt.node = n.id')
+ ->where('nt.lang = :lang AND n.deleted != 1');
+
+ $this->setExpectedException('Doctrine\DBAL\Query\QueryException', "The given alias 'invalid' is not part of any FROM or JOIN clause table. The currently registered aliases are: news, nv, nt, n.");
$this->assertEquals('', $qb->getSQL());
}
+
+ /**
+ * @group DBAL-172
+ */
+ public function testSelectFromMasterWithWhereOnJoinedTables()
+ {
+ $qb = new QueryBuilder($this->conn);
+
+ $qb->select('COUNT(DISTINCT news.id)')
+ ->from('newspages', 'news')
+ ->innerJoin('news', 'nodeversion', 'nv', "nv.refId = news.id AND nv.refEntityname='Entity\\News'")
+ ->innerJoin('nv', 'nodetranslation', 'nt', 'nv.nodetranslation = nt.id')
+ ->innerJoin('nt', 'node', 'n', 'nt.node = n.id')
+ ->where('nt.lang = ?')
+ ->andWhere('n.deleted = 0');
+
+ $this->assertEquals("SELECT COUNT(DISTINCT news.id) FROM newspages news INNER JOIN nodeversion nv ON nv.refId = news.id AND nv.refEntityname='Entity\\News' INNER JOIN nodetranslation nt ON nv.nodetranslation = nt.id INNER JOIN node n ON nt.node = n.id WHERE (nt.lang = ?) AND (n.deleted = 0)", $qb->getSQL());
+ }
}
Something went wrong with that request. Please try again.