Quote strategy not applied to foreign key columns

[lemartin]

The quote strategy is not applied to foreign key columns used in one-to-one associations. When fetching the Address entity the column user-id is not quoted.

SELECT 
  q0_."address-id" AS addressid_0,
  q0_."address-zip" AS addresszip_1,
  q0_.user-id AS userid_2
FROM "quote-address" q0_

The tests do not cover this scenario, as they force partially loaded entities. Applying the patch below should yield a failing test case. The columns come from a collection called targetToSourceKeyColumns where, if I understood correctly, all columns are stored that are used as

As far as I can tell, the problem is in SqlWalker::walkSelectClause, where the foreign key columns are added to the list of select expressions.

Patch for creating a test case, based on branch 2.5 (48e8c02):

diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
index a3a34f8..21b6961 100644
--- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
+++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
@@ -1972,0 +1973,6 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase
+
+        $this->assertSqlGeneration(
+            'SELECT a FROM Doctrine\Tests\Models\Quote\Address a',
+            'SELECT q0_."address-id" AS addressid_0, q0_."address-zip" AS addresszip_1, q0_."user-id" AS userid_2 FROM "quote-address" q0_',
+            [Query::HINT_FORCE_PARTIAL_LOAD => false]
+        );

[theofidry]

AFAIKS it's a duplicate of #5874 :/

[Ocramius]

@theofidry it's similar, but not a duplicate. @lemartin is the join column mapped with backticks?

[lemartin]

Yes, the column is mapped with back-ticks.

    /**
     * @OneToOne(targetEntity="User", inversedBy="address")
     * @JoinColumn(name="`user-id`", referencedColumnName="`user-id`")
     */
    public $user;

[Ocramius]

Yup, that's 100% a bug then, and not a duplicate :-\

[lemartin]

After some debugging I think I found a fix, but also another bug. See below the diff for fixing the original issue. I'm not sure if the map is really necessary, or if there is an easier way to find the joinColumn based on a column name (as found in targetToSourceKeyColumns).

What I am really unsure about is, what is supposed to go into ResultSetMapping::addMetaResult, whether it should be the quoted column name or the column name as per joinColumn and targetToSourceKeyColumns. The thing is, it only works with the unquoted column name, and if this is as it is supposed, there is probably a bug in BasicEntityPersister::_getSelectColumnAssociationSQL introduced in a75c672.

There the quoted column name is passed to ResultSetMapping::addMetaResult, causing a undefined index notice in IdentifierFlattener::flattenIdentifier and subsequently the following error: The given entity of type 'Foo' (DoctrineProxies\__CG__\Foo@000000002cd49270000000000c8857e4) has no identity/no id values set. It cannot be added to the identity map. .

The tests are no help, they succeed regardless if the quoted or unquoted column name is passed.

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
index 9c4ac2a..5e5ae62 100644
--- a/lib/Doctrine/ORM/Query/SqlWalker.php
+++ b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -756,12 +756,18 @@ class SqlWalker implements TreeWalker
 
                 $targetClass = $this->em->getClassMetadata($assoc['targetEntity']);
 
+                $columnNameToJoinColumn = [];
+                foreach($assoc['joinColumns'] as &$joinColumn) {
+                    $columnNameToJoinColumn[$joinColumn['name']] = $joinColumn;
+                }
+
                 foreach ($assoc['targetToSourceKeyColumns'] as $targetColumn => $srcColumn) {
                     $columnAlias = $this->getSQLColumnAlias($srcColumn);
+                    $quotedSourceColumn = $this->quoteStrategy->getJoinColumnName($columnNameToJoinColumn[$srcColumn], $class, $this->platform);
 
                     $type                   = null;
                     $isIdentifier           = (isset($assoc['id']) && $assoc['id'] === true);
-                    $sqlSelectExpressions[] = $sqlTableAlias . '.' . $srcColumn . ' AS ' . $columnAlias;
+                    $sqlSelectExpressions[] = $sqlTableAlias . '.' . $quotedSourceColumn . ' AS ' . $columnAlias;
 
                     if (isset($targetClass->fieldNames[$targetColumn])) {
                         $type = $targetClass->fieldMappings[$targetClass->fieldNames[$targetColumn]]['type'];

[lemartin]

Created a pull request for both issues. (My first ever, sorry if I got anything wrong).

The PR targets the master, how are the chances, given a PR for 2.5 would be supplied, that this fix could find its way into a 2.5.x?

[lcobucci]

@lemartin once the PR is merged be me or @Ocramius we'll do the backporting 😉

[lcobucci]

@lemartin thanks for your contribution as explained here we're going to keep it on v2.6.x only.

[lcobucci]

Closed by #6404

[Ocramius]

Handled in #6416