Query builder yields invalid SQL with JOINED inheritance type on PostgreSQL

[stesie]

Given a simple set of entities (adapted to a simpler domain here):

• a Post belongs to an Author which derives from User
• the inheritance is mapped by JOINED type
• yet the relation between Post and Author is not mapped (for reasons, as the real case is less simple)

<?php
namespace Entity;

/**
 * @Entity @Table(name="posts")
 */
class Post
{
    /** @Id @GeneratedValue @Column(type="integer") */
    public $id;

    /** @Column(type="integer") */
    public $authorId;

    /** @Column(length=100) */
    public $title;

    /** @Column(type="text") */
    public $text;
}

<?php
namespace Entity;

/**
 * @Entity @Table(name="authors")
 */
class Author extends User
{
    /** @Column(length=50) */
    public $displayName;
}

<?php
namespace Entity;

/**
 * @Entity @Table(name="users")
 * @InheritanceType("JOINED")
 * @DiscriminatorColumn(name="discr", type="string")
 * @DiscriminatorMap({"author" = "Author"})
 */
abstract class User
{
    /** @Id @GeneratedValue @Column(type="integer") */
    public $id;

    /** @Column(length=50) */
    public $username;

    /** @Column(length=100) */
    public $password;
}

If I then instanciate and use a QueryBuilder like this:

$qb = $entityManager->createQueryBuilder();

$qb
    ->select('p', 'a')
    ->from('Entity\\Post', 'p')
    ->innerJoin('Entity\\Author', 'a', 'WITH', 'p.authorId = a.id')
    ;

it generates the following SQL

SELECT ... FROM posts p0_ INNER JOIN authors a2_ INNER JOIN users u1_ ON a2_.id = u1_.id AND (p0_.authorId = u1_.id)

Which is valid for MySQL, however is not valid for PostgreSQL (tried with 9.6)

PostgreSQL seems to require an ON clause with each and every INNER JOIN (interestingly as opposed to LEFT JOIN). This is, the following query works with it:

SELECT ... FROM posts p0_ INNER JOIN users u1_ ON (p0_.authorId = u1_.id) INNER JOIN authors a2_ ON (a2_.id = u1_.id) ;

Pushing all conditions to WHERE also works on PostgreSQL:

SELECT ... FROM posts p0_ , authors a2_ , users u1_ WHERE (a2_.id = u1_.id)  AND  (p0_.authorId = u1_.id);

I haven't yet dug deeper, mainly as I never dived into Doctrine ORM code ... please let me know if you agree on this being a bug, ... I so far have no idea where it goes wrong, but let me know if I can support you, maybe by providing test code or start digging ...

[stesie]

Well, so I had another look at this issue myself, and the SQL that probably should be generated is

SELECT ... FROM posts p0_ INNER JOIN (authors a2_ INNER JOIN users u1_ ON a2_.id = u1_.id) ON (p0_.authorId = u1_.id)

... mind the parentheses around the CTI JOIN. SqlWalker::walkJoinAssociationDeclaration already has similar syntax around line 1043:

        if ($targetClass->isInheritanceTypeJoined()) {
            $ctiJoins = $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias);
            // If we have WITH condition, we need to build nested joins for target class table and cti joins
            if ($withCondition) {
                $sql .= '(' . $targetTableJoin['table'] . $ctiJoins . ') ON ' . $targetTableJoin['condition'];
            } else {
                $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition'] . $ctiJoins;
            }
        } else {
            $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition'];
        }

If I apply this also to the code path through walkJoin and walkRangeVariableDeclaration like this:

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
index b39280b39..98c30c195 100644
--- a/lib/Doctrine/ORM/Query/SqlWalker.php
+++ b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -890,7 +890,7 @@ public function walkRangeVariableDeclaration($rangeVariableDeclaration)
         );
 
         if ($class->isInheritanceTypeJoined()) {
-            $sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
+            $sql = '(' . $sql . $this->_generateClassTableInheritanceJoins($class, $dqlAlias) . ')';
         }
 
         return $sql;
@@ -1136,10 +1136,6 @@ public function walkJoin($join)
                     $conditions[] = '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')';
                 }
 
-                $condExprConjunction = ($class->isInheritanceTypeJoined() && $joinType != AST\Join::JOIN_TYPE_LEFT && $joinType != AST\Join::JOIN_TYPE_LEFTOUTER)
-                    ? ' AND '
-                    : ' ON ';
-
                 $sql .= $this->walkRangeVariableDeclaration($joinDeclaration);
 
                 // Apply remaining inheritance restrictions
@@ -1157,7 +1153,7 @@ public function walkJoin($join)
                 }
 
                 if ($conditions) {
-                    $sql .= $condExprConjunction . implode(' AND ', $conditions);
+                    $sql .= ' ON ' . implode(' AND ', $conditions);
                 }
 
                 break;

... the sql statement from above is produced. And it works fine with PostgreSQL as well as MySQL.

Let me know if that's the way to go, then I'll happily provide a pull request...

[Deltachaos]

This change is causing unnecessary bracets if there is no other JOIN in the brackets. This is invalid syntax (at least for SQL Server). Please have a look at the fix #6812