Merge pull request #3382 from morozov/issues/3377

Reverted strict comparison back to loose because of a new regression

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -435,7 +435,7 @@ class Comparator
         // Null values need to be checked additionally as they tell whether to create or drop a default value.
         // null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation.
         if (($properties1['default'] === null) !== ($properties2['default'] === null)
-            || (string) $properties1['default'] !== (string) $properties2['default']) {
+            || $properties1['default'] != $properties2['default']) {
             $changedProperties[] = 'default';
         }
 

lib/Doctrine/DBAL/Schema/OracleSchemaManager.php

@@ -128,7 +128,7 @@ class OracleSchemaManager extends AbstractSchemaManager
             }
         }
 
-        $unsigned = $fixed = null;
+        $unsigned = $fixed = $precision = $scale = $length = null;
 
         if (! isset($tableColumn['column_name'])) {
             $tableColumn['column_name'] = '';
@@ -146,8 +146,13 @@ class OracleSchemaManager extends AbstractSchemaManager
             $tableColumn['data_default'] = trim($tableColumn['data_default'], "'");
         }
 
-        $precision = null;
-        $scale     = null;
+        if ($tableColumn['data_precision'] !== null) {
+            $precision = (int) $tableColumn['data_precision'];
+        }
+
+        if ($tableColumn['data_scale'] !== null) {
+            $scale = (int) $tableColumn['data_scale'];
+        }
 
         $type                    = $this->_platform->getDoctrineTypeMapping($dbType);
         $type                    = $this->extractDoctrineTypeFromComment($tableColumn['comments'], $type);
@@ -155,28 +160,16 @@ class OracleSchemaManager extends AbstractSchemaManager
 
         switch ($dbType) {
             case 'number':
-                if ($tableColumn['data_precision'] === 20 && $tableColumn['data_scale'] === 0) {
-                    $precision = 20;
-                    $scale     = 0;
-                    $type      = 'bigint';
-                } elseif ($tableColumn['data_precision'] === 5 && $tableColumn['data_scale'] === 0) {
-                    $type      = 'smallint';
-                    $precision = 5;
-                    $scale     = 0;
-                } elseif ($tableColumn['data_precision'] === 1 && $tableColumn['data_scale'] === 0) {
-                    $precision = 1;
-                    $scale     = 0;
-                    $type      = 'boolean';
-                } elseif ($tableColumn['data_scale'] > 0) {
-                    $precision = $tableColumn['data_precision'];
-                    $scale     = $tableColumn['data_scale'];
-                    $type      = 'decimal';
+                if ($precision === 20 && $scale === 0) {
+                    $type = 'bigint';
+                } elseif ($precision === 5 && $scale === 0) {
+                    $type = 'smallint';
+                } elseif ($precision === 1 && $scale === 0) {
+                    $type = 'boolean';
+                } elseif ($scale > 0) {
+                    $type = 'decimal';
                 }
-                $length = null;
-                break;
-            case 'pls_integer':
-            case 'binary_integer':
-                $length = null;
+
                 break;
             case 'varchar':
             case 'varchar2':
@@ -189,31 +182,6 @@ class OracleSchemaManager extends AbstractSchemaManager
                 $length = $tableColumn['char_length'];
                 $fixed  = true;
                 break;
-            case 'date':
-            case 'timestamp':
-                $length = null;
-                break;
-            case 'float':
-            case 'binary_float':
-            case 'binary_double':
-                $precision = $tableColumn['data_precision'];
-                $scale     = $tableColumn['data_scale'];
-                $length    = null;
-                break;
-            case 'clob':
-            case 'nclob':
-                $length = null;
-                break;
-            case 'blob':
-            case 'raw':
-            case 'long raw':
-            case 'bfile':
-                $length = null;
-                break;
-            case 'rowid':
-            case 'urowid':
-            default:
-                $length = null;
         }
 
         $options = [

phpcs.xml.dist

@@ -58,4 +58,9 @@
         <exclude-pattern>tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php</exclude-pattern>
         <exclude-pattern>tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php</exclude-pattern>
     </rule>
+
+    <!-- see https://github.com/doctrine/dbal/issues/3377 -->
+    <rule ref="SlevomatCodingStandard.ControlStructures.DisallowEqualOperators.DisallowedNotEqualOperator">
+        <exclude-pattern>lib/Doctrine/DBAL/Schema/Comparator.php</exclude-pattern>
+    </rule>
 </ruleset>

tests/Doctrine/Tests/DBAL/Functional/Schema/ComparatorTest.php

@@ -25,15 +25,31 @@ class ComparatorTest extends DbalFunctionalTestCase
         $this->comparator    = new Comparator();
     }
 
-    public function testDefaultValueComparison()
+    /**
+     * @param mixed $value
+     *
+     * @dataProvider defaultValueProvider
+     */
+    public function testDefaultValueComparison(string $type, $value) : void
     {
         $table = new Table('default_value');
-        $table->addColumn('id', 'integer', ['default' => 1]);
+        $table->addColumn('test', $type, ['default' => $value]);
 
-        $this->schemaManager->createTable($table);
+        $this->schemaManager->dropAndCreateTable($table);
 
         $onlineTable = $this->schemaManager->listTableDetails('default_value');
 
         self::assertFalse($this->comparator->diffTable($table, $onlineTable));
     }
+
+    /**
+     * @return mixed[][]
+     */
+    public static function defaultValueProvider() : iterable
+    {
+        return [
+            ['integer', 1],
+            ['boolean', false],
+        ];
+    }
 }