Trigger a notice when removing aliases from queries with joins.

Removing aliases from query conditions can break references to joins. Since joins for delete/update queries aren't yet supported, this notice is ment to serve as a hint for future implementations.
cakephp · Sep 8, 2016 · 969c8c6 · 969c8c6
1 parent d3a84aa
commit 969c8c6
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 0 deletions.
diff --git a/src/Database/SqlDialectTrait.php b/src/Database/SqlDialectTrait.php
@@ -206,6 +206,14 @@ protected function _updateQueryTranslator($query)
      */
     protected function _removeAliasesFromConditions($query)
     {
+        if (!empty($query->clause('join'))) {
+            trigger_error(
+                'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
+                'this can break references to joined tables.',
+                E_USER_NOTICE
+            );
+        }
+
         $conditions = $query->clause('where');
         if ($conditions) {
             $conditions->traverse(function ($condition) {

diff --git a/tests/TestCase/Database/QueryTest.php b/tests/TestCase/Database/QueryTest.php
@@ -2650,6 +2650,41 @@ public function testDeleteNoFrom()
         $result->closeCursor();
     }
 
+    /**
+     * Tests that delete queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     * @return void
+     */
+    public function testDeleteRemovingAliasesCanBreakJoins()
+    {
+        $message = null;
+        $oldHandler = set_error_handler(function ($errno, $errstr) use (&$oldHandler, &$message) {
+            if ($errno === E_USER_NOTICE) {
+                $message = $errstr;
+            } else {
+                call_user_func_array($oldHandler, func_get_args());
+            }
+        });
+
+        $query = new Query($this->connection);
+
+        $query
+            ->delete('authors')
+            ->from(['a ' => 'authors'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+
+        restore_error_handler();
+
+        $expected = 'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
+            'this can break references to joined tables.';
+        $this->assertEquals($expected, $message);
+    }
+
     /**
      * Test setting select() & delete() modes.
      *
@@ -2873,6 +2908,41 @@ public function testUpdateStripAliasesFromConditions()
         );
     }
 
+    /**
+     * Tests that update queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     * @return void
+     */
+    public function testUpdateRemovingAliasesCanBreakJoins()
+    {
+        $message = null;
+        $oldHandler = set_error_handler(function ($errno, $errstr) use (&$oldHandler, &$message) {
+            if ($errno === E_USER_NOTICE) {
+                $message = $errstr;
+            } else {
+                call_user_func_array($oldHandler, func_get_args());
+            }
+        });
+
+        $query = new Query($this->connection);
+
+        $query
+            ->update('authors')
+            ->set(['name' => 'name'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+
+        restore_error_handler();
+
+        $expected = 'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
+            'this can break references to joined tables.';
+        $this->assertEquals($expected, $message);
+    }
+
     /**
      * You cannot call values() before insert() it causes all sorts of pain.
      *