Merge pull request #15511 from ndm2/3.x-tuple-comparison-type-handling

3.x - Fix tuple comparison type handling

src/Database/Dialect/TupleComparisonTranslatorTrait.php

@@ -69,6 +69,13 @@ trait TupleComparisonTranslatorTrait
             return;
         }
 
+        $type = $expression->getType();
+        if ($type) {
+            $typeMap = array_combine($fields, $type);
+        } else {
+            $typeMap = [];
+        }
+
         $surrogate = $query->getConnection()
             ->newQuery()
             ->select($true);
@@ -85,7 +92,7 @@ trait TupleComparisonTranslatorTrait
             }
             $conditions['OR'][] = $item;
         }
-        $surrogate->where($conditions);
+        $surrogate->where($conditions, $typeMap);
 
         $expression->setField($true);
         $expression->setValue($surrogate);

src/Database/Expression/TupleComparison.php

@@ -46,6 +46,16 @@ class TupleComparison extends Comparison
     }
 
     /**
+     * Returns the type to be used for casting the value to a database representation
+     *
+     * @return array
+     */
+    public function getType()
+    {
+        return $this->_type;
+    }
+
+    /**
      * Convert the expression into a SQL fragment.
      *
      * @param \Cake\Database\ValueBinder $generator Placeholder generator object

src/ORM/Marshaller.php

@@ -496,7 +496,12 @@ class Marshaller
             if (!is_array($first) || count($first) !== count($primaryKey)) {
                 return [];
             }
-            $filter = new TupleComparison($primaryKey, $ids, [], 'IN');
+            $type = [];
+            $schema = $target->getSchema();
+            foreach ((array)$target->getPrimaryKey() as $column) {
+                $type[] = $schema->getColumnType($column);
+            }
+            $filter = new TupleComparison($primaryKey, $ids, $type, 'IN');
         } else {
             $filter = [$primaryKey[0] . ' IN' => $ids];
         }

tests/TestCase/Database/QueryTest.php

@@ -17,6 +17,7 @@ namespace Cake\Test\TestCase\Database;
 use Cake\Database\ExpressionInterface;
 use Cake\Database\Expression\IdentifierExpression;
 use Cake\Database\Expression\QueryExpression;
+use Cake\Database\Expression\TupleComparison;
 use Cake\Database\Query;
 use Cake\Database\StatementInterface;
 use Cake\Database\Statement\StatementDecorator;
@@ -3884,6 +3885,87 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that the values in tuple comparison expression are being bound correctly,
+     * specifically for dialects that translate tuple comparisons.
+     *
+     * @return void
+     * @see \Cake\Database\Dialect\TupleComparisonTranslatorTrait::_transformTupleComparison()
+     * @see \Cake\Database\Driver\Sqlite::_expressionTranslators()
+     * @see \Cake\Database\Driver\Sqlserver::_expressionTranslators()
+     */
+    public function testTupleComparisonValuesAreBeingBoundCorrectly()
+    {
+        // Load with force dropping tables to avoid identities not being reset properly
+        // in SQL Server when reseeding is applied directly after table creation.
+        $this->fixtureManager->loadSingle('Profiles', null, true);
+
+        $profiles = $this->getTableLocator()->get('Profiles');
+
+        $query = $profiles
+            ->find()
+            ->where(
+                new TupleComparison(
+                    ['id', 'user_id'],
+                    [[1, 1]],
+                    ['integer', 'integer'],
+                    'IN'
+                )
+            );
+
+        $result = $query->firstOrFail();
+
+        $bindings = array_values($query->getValueBinder()->bindings());
+        $this->assertCount(2, $bindings);
+        $this->assertSame(1, $bindings[0]['value']);
+        $this->assertSame('integer', $bindings[0]['type']);
+        $this->assertSame(1, $bindings[1]['value']);
+        $this->assertSame('integer', $bindings[1]['type']);
+
+        $this->assertSame(1, $result['id']);
+        $this->assertSame(1, $result['user_id']);
+    }
+
+    /**
+     * Tests that the values in tuple comparison expressions are being bound as expected
+     * when types are omitted, specifically for dialects that translate tuple comparisons.
+     *
+     * @return void
+     * @see \Cake\Database\Dialect\TupleComparisonTranslatorTrait::_transformTupleComparison()
+     * @see \Cake\Database\Driver\Sqlite::_expressionTranslators()
+     * @see \Cake\Database\Driver\Sqlserver::_expressionTranslators()
+     */
+    public function testTupleComparisonTypesCanBeOmitted()
+    {
+        // Load with force dropping tables to avoid identities not being reset properly
+        // in SQL Server when reseeding is applied directly after table creation.
+        $this->fixtureManager->loadSingle('Profiles', null, true);
+
+        $profiles = $this->getTableLocator()->get('Profiles');
+
+        $query = $profiles
+            ->find()
+            ->where(
+                new TupleComparison(
+                    ['id', 'user_id'],
+                    [[1, 1]],
+                    [],
+                    'IN'
+                )
+            );
+        $result = $query->firstOrFail();
+
+        $bindings = array_values($query->getValueBinder()->bindings());
+        $this->assertCount(2, $bindings);
+        $this->assertSame(1, $bindings[0]['value']);
+        $this->assertNull($bindings[0]['type']);
+        $this->assertSame(1, $bindings[1]['value']);
+        $this->assertNull($bindings[1]['type']);
+
+        $this->assertSame(1, $result['id']);
+        $this->assertSame(1, $result['user_id']);
+    }
+
+    /**
      * Tests that default types are passed to functions accepting a $types param
      *
      * @return void

tests/TestCase/ORM/MarshallerTest.php

@@ -3558,4 +3558,46 @@ class MarshallerTest extends TestCase
         ];
         $this->assertEquals($expected, $result->toArray());
     }
+
+    /**
+     * Tests that ID values are being bound with the correct type when loading associated records.
+     */
+    public function testInvalidTypesWhenLoadingAssociatedByIds()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot convert value of type `string` to integer');
+
+        $data = [
+            'title' => 'article',
+            'body' => 'some content',
+            'comments' => [
+                '_ids' => ['foobar'],
+            ],
+        ];
+
+        $marshaller = new Marshaller($this->articles);
+        $marshaller->one($data, ['associated' => ['Comments']]);
+    }
+
+    /**
+     * Tests that composite ID values are being bound with the correct type when loading associated records.
+     */
+    public function testInvalidTypesWhenLoadingAssociatedByCompositeIds()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot convert value of type `string` to integer');
+
+        $data = [
+            'title' => 'article',
+            'body' => 'some content',
+            'comments' => [
+                '_ids' => [['foo', 'bar']],
+            ],
+        ];
+
+        $this->articles->Comments->setPrimaryKey(['id', 'article_id']);
+
+        $marshaller = new Marshaller($this->articles);
+        $marshaller->one($data, ['associated' => ['Comments']]);
+    }
 }