Browse Source

Merge pull request #9420 from ndm2/fix-strip-update-condition-aliases

3.x - Strip aliases from update query conditions where possible.
Mark Story 9 years ago
parent
commit
8a39442295
2 changed files with 122 additions and 11 deletions
  1. 37 11
      src/Database/SqlDialectTrait.php
  2. 85 0
      tests/TestCase/Database/QueryTest.php

+ 37 - 11
src/Database/SqlDialectTrait.php

@@ -178,6 +178,43 @@ trait SqlDialectTrait
         if (!$hadAlias) {
             return $query;
         }
+
+        return $this->_removeAliasesFromConditions($query);
+    }
+
+    /**
+     * Apply translation steps to update queries.
+     *
+     * Chops out aliases on update query conditions as not all database dialects do support
+     * aliases in update queries.
+     *
+     * Just like for delete queries, joins are currently not supported for update queries.
+     *
+     * @param \Cake\Database\Query $query The query to translate
+     * @return \Cake\Database\Query The modified query
+     */
+    protected function _updateQueryTranslator($query)
+    {
+        return $this->_removeAliasesFromConditions($query);
+    }
+
+    /**
+     * Removes aliases from the `WHERE` clause of a query.
+     *
+     * @param \Cake\Database\Query $query The query to process.
+     * @return \Cake\Database\Query The modified query.
+     * @throws \RuntimeException In case the processed query contains any joins, as removing
+     *  aliases from the conditions can break references to the joined tables.
+     */
+    protected function _removeAliasesFromConditions($query)
+    {
+        if ($query->clause('join')) {
+            throw new \RuntimeException(
+                'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
+                'this can break references to joined tables.'
+            );
+        }
+
         $conditions = $query->clause('where');
         if ($conditions) {
             $conditions->traverse(function ($condition) {
@@ -201,17 +238,6 @@ trait SqlDialectTrait
     }
 
     /**
-     * Apply translation steps to update queries.
-     *
-     * @param \Cake\Database\Query $query The query to translate
-     * @return \Cake\Database\Query The modified query
-     */
-    protected function _updateQueryTranslator($query)
-    {
-        return $query;
-    }
-
-    /**
      * Apply translation steps to insert queries.
      *
      * @param \Cake\Database\Query $query The query to translate

+ 85 - 0
tests/TestCase/Database/QueryTest.php

@@ -2671,6 +2671,29 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that delete queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     *
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
+     * @return void
+     */
+    public function testDeleteRemovingAliasesCanBreakJoins()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->delete('authors')
+            ->from(['a ' => 'authors'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+    }
+
+    /**
      * Test setting select() & delete() modes.
      *
      * @return void
@@ -2854,6 +2877,68 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that aliases are stripped from update query conditions
+     * where possible.
+     *
+     * @return void
+     */
+    public function testUpdateStripAliasesFromConditions()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->update('authors')
+            ->set(['name' => 'name'])
+            ->where([
+                'OR' => [
+                    'a.id' => 1,
+                    'AND' => [
+                        'b.name NOT IN' => ['foo', 'bar'],
+                        'OR' => [
+                            $query->newExpr()->eq(new IdentifierExpression('c.name'), 'zap'),
+                            'd.name' => 'baz',
+                            (new Query($this->connection))->select(['e.name'])->where(['e.name' => 'oof'])
+                        ]
+                    ]
+                ],
+            ]);
+
+        $this->assertQuotedQuery(
+            'UPDATE <authors> SET <name> = :c0 WHERE \(' .
+                '<id> = :c1 OR \(' .
+                    '<name> not in \(:c2,:c3\) AND \(' .
+                        '\(<c>\.<name>\) = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
+                    '\)' .
+                '\)' .
+            '\)',
+            $query->sql(),
+            !$this->autoQuote
+        );
+    }
+
+    /**
+     * Tests that update queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
+     * @return void
+     */
+    public function testUpdateRemovingAliasesCanBreakJoins()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->update('authors')
+            ->set(['name' => 'name'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+    }
+
+    /**
      * You cannot call values() before insert() it causes all sorts of pain.
      *
      * @expectedException \Cake\Database\Exception