Merge pull request #819 from hason/sqlite_comment

Added support for inline comments for SQLite

fixes #1118

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -1608,6 +1608,24 @@ abstract class AbstractPlatform
             " IS " . $comment;
     }
 
+    /**
+     * Returns the SQL to create inline comment on a column.
+     *
+     * @param string $comment
+     *
+     * @return string
+     *
+     * @throws \Doctrine\DBAL\DBALException If not supported on this platform.
+     */
+    public function getInlineColumnCommentSQL($comment)
+    {
+        if (! $this->supportsInlineColumnComments()) {
+            throw DBALException::notSupported(__METHOD__);
+        }
+
+        return "COMMENT " . $this->quoteStringLiteral($comment);
+    }
+
     /**
      * Returns the SQL used to create a table.
      *
@@ -2217,7 +2235,7 @@ abstract class AbstractPlatform
         }
 
         if ($this->supportsInlineColumnComments() && isset($field['comment']) && $field['comment'] !== '') {
-            $columnDef .= " COMMENT " . $this->quoteStringLiteral($field['comment']);
+            $columnDef .= ' ' . $this->getInlineColumnCommentSQL($field['comment']);
         }
 
         return $name . ' ' . $columnDef;

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -492,6 +492,14 @@ class SqlitePlatform extends AbstractPlatform
         return true;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function supportsInlineColumnComments()
+    {
+        return true;
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -567,6 +575,14 @@ class SqlitePlatform extends AbstractPlatform
         return '';
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function getInlineColumnCommentSQL($comment)
+    {
+        return '--' . str_replace("\n", "\n--", $comment) . "\n";
+    }
+
     /**
      * {@inheritDoc}
      */

lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php

@@ -22,6 +22,7 @@ namespace Doctrine\DBAL\Schema;
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Types\StringType;
 use Doctrine\DBAL\Types\TextType;
+use Doctrine\DBAL\Types\Type;
 
 /**
  * Sqlite SchemaManager.
@@ -118,6 +119,7 @@ class SqliteSchemaManager extends AbstractSchemaManager
         if ( ! empty($tableForeignKeys)) {
             $createSql = $this->_conn->fetchAll("SELECT sql FROM (SELECT * FROM sqlite_master UNION ALL SELECT * FROM sqlite_temp_master) WHERE type = 'table' AND name = '$table'");
             $createSql = isset($createSql[0]['sql']) ? $createSql[0]['sql'] : '';
+
             if (preg_match_all('#
                     (?:CONSTRAINT\s+([^\s]+)\s+)?
                     (?:FOREIGN\s+KEY[^\)]+\)\s*)?
@@ -168,6 +170,7 @@ class SqliteSchemaManager extends AbstractSchemaManager
         // fetch primary
         $stmt = $this->_conn->executeQuery("PRAGMA TABLE_INFO ('$tableName')");
         $indexArray = $stmt->fetchAll(\PDO::FETCH_ASSOC);
+
         usort($indexArray, function($a, $b) {
             if ($a['pk'] == $b['pk']) {
                 return $a['cid'] - $b['cid'];
@@ -248,7 +251,7 @@ class SqliteSchemaManager extends AbstractSchemaManager
             }
         }
 
-        // inspect column collation
+        // inspect column collation and comments
         $createSql = $this->_conn->fetchAll("SELECT sql FROM (SELECT * FROM sqlite_master UNION ALL SELECT * FROM sqlite_temp_master) WHERE type = 'table' AND name = '$table'");
         $createSql = isset($createSql[0]['sql']) ? $createSql[0]['sql'] : '';
 
@@ -258,6 +261,20 @@ class SqliteSchemaManager extends AbstractSchemaManager
             if ($type instanceof StringType || $type instanceof TextType) {
                 $column->setPlatformOption('collation', $this->parseColumnCollationFromSQL($columnName, $createSql) ?: 'BINARY');
             }
+
+            $comment = $this->parseColumnCommentFromSQL($columnName, $createSql);
+
+            if (false !== $comment) {
+                $type = $this->extractDoctrineTypeFromComment($comment, null);
+
+                if (null !== $type) {
+                    $column->setType(Type::getType($type));
+
+                    $comment = $this->removeDoctrineTypeFromComment($comment, $type);
+                }
+
+                $column->setComment($comment);
+            }
         }
 
         return $list;
@@ -436,4 +453,25 @@ class SqliteSchemaManager extends AbstractSchemaManager
 
         return false;
     }
+
+    /**
+     * @param string $column
+     * @param string $sql
+     *
+     * @return string|false
+     */
+    private function parseColumnCommentFromSQL($column, $sql)
+    {
+        if (preg_match(
+            '{[\s(,](?:'.preg_quote($this->_platform->quoteSingleIdentifier($column)).'|'.preg_quote($column).')
+            (?:\(.*?\)|[^,(])*?,?((?:\s*--[^\n]*\n?)+)
+            }isx', $sql, $match
+        )) {
+            $comment = preg_replace('{^\s*--}m', '', rtrim($match[1], "\n"));
+
+            return '' === $comment ? false : $comment;
+        }
+
+        return false;
+    }
 }

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

@@ -701,6 +701,7 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest
         $this->assertEquals('This is a comment', $columns['id']->getComment());
 
         $tableDiff = new \Doctrine\DBAL\Schema\TableDiff('column_comment_test');
+        $tableDiff->fromTable = $table;
         $tableDiff->changedColumns['id'] = new \Doctrine\DBAL\Schema\ColumnDiff(
             'id', new \Doctrine\DBAL\Schema\Column(
                 'id', \Doctrine\DBAL\Types\Type::getType('integer'), array('primary' => true)
@@ -1053,7 +1054,6 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest
         $offlineTable->addColumn('comment2', 'integer', array('comment' => $comment2));
         $offlineTable->addColumn('no_comment1', 'integer');
         $offlineTable->addColumn('no_comment2', 'integer');
-
         $this->_sm->dropAndCreateTable($offlineTable);
 
         $onlineTable = $this->_sm->listTableDetails("alter_column_comment_test");

tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php

@@ -1073,6 +1073,55 @@ abstract class AbstractPlatformTestCase extends \Doctrine\Tests\DbalTestCase
         );
     }
 
+    /**
+     * @group DBAL-1176
+     *
+     * @dataProvider getGeneratesInlineColumnCommentSQL
+     */
+    public function testGeneratesInlineColumnCommentSQL($comment, $expectedSql)
+    {
+        if (! $this->_platform->supportsInlineColumnComments()) {
+            $this->markTestSkipped(sprintf('%s does not support inline column comments.', get_class($this->_platform)));
+        }
+
+        $this->assertSame($expectedSql, $this->_platform->getInlineColumnCommentSQL($comment));
+    }
+
+    public function getGeneratesInlineColumnCommentSQL()
+    {
+        return array(
+            'regular comment' => array('Regular comment', $this->getInlineColumnRegularCommentSQL()),
+            'comment requiring escaping' => array(
+                sprintf(
+                    'Using inline comment delimiter %s works',
+                    $this->getInlineColumnCommentDelimiter()
+                ),
+                $this->getInlineColumnCommentRequiringEscapingSQL()
+            ),
+            'empty comment' => array('', $this->getInlineColumnEmptyCommentSQL()),
+        );
+    }
+
+    protected function getInlineColumnCommentDelimiter()
+    {
+        return "'";
+    }
+
+    protected function getInlineColumnRegularCommentSQL()
+    {
+        return "COMMENT 'Regular comment'";
+    }
+
+    protected function getInlineColumnCommentRequiringEscapingSQL()
+    {
+        return "COMMENT 'Using inline comment delimiter '' works'";
+    }
+
+    protected function getInlineColumnEmptyCommentSQL()
+    {
+        return "COMMENT ''";
+    }
+
     protected function getQuotedStringLiteralWithoutQuoteCharacter()
     {
         return "'No quote'";
@@ -1088,6 +1137,24 @@ abstract class AbstractPlatformTestCase extends \Doctrine\Tests\DbalTestCase
         return "''''";
     }
 
+    /**
+     * @group DBAL-1176
+     */
+    public function testThrowsExceptionOnGeneratingInlineColumnCommentSQLIfUnsupported()
+    {
+        if ($this->_platform->supportsInlineColumnComments()) {
+            $this->markTestSkipped(sprintf('%s supports inline column comments.', get_class($this->_platform)));
+        }
+
+        $this->setExpectedException(
+            'Doctrine\DBAL\DBALException',
+            "Operation 'Doctrine\\DBAL\\Platforms\\AbstractPlatform::getInlineColumnCommentSQL' is not supported by platform.",
+            0
+        );
+
+        $this->_platform->getInlineColumnCommentSQL('unsupported');
+    }
+
     public function testQuoteStringLiteral()
     {
         $c = $this->getStringLiteralQuoteCharacter();

tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php

@@ -523,11 +523,20 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
         return array(
             'CREATE TEMPORARY TABLE __temp__mytable AS SELECT unquoted1, unquoted2, unquoted3, "create", "table", "select", "quoted1", "quoted2", "quoted3" FROM mytable',
             'DROP TABLE mytable',
-            'CREATE TABLE mytable (unquoted INTEGER NOT NULL, "where" INTEGER NOT NULL, "foo" INTEGER NOT NULL, reserved_keyword INTEGER NOT NULL, "from" INTEGER NOT NULL, "bar" INTEGER NOT NULL, quoted INTEGER NOT NULL, "and" INTEGER NOT NULL, "baz" INTEGER NOT NULL)',
+            'CREATE TABLE mytable (unquoted INTEGER NOT NULL --Unquoted 1
+, "where" INTEGER NOT NULL --Unquoted 2
+, "foo" INTEGER NOT NULL --Unquoted 3
+, reserved_keyword INTEGER NOT NULL --Reserved keyword 1
+, "from" INTEGER NOT NULL --Reserved keyword 2
+, "bar" INTEGER NOT NULL --Reserved keyword 3
+, quoted INTEGER NOT NULL --Quoted 1
+, "and" INTEGER NOT NULL --Quoted 2
+, "baz" INTEGER NOT NULL --Quoted 3
+)',
             'INSERT INTO mytable (unquoted, "where", "foo", reserved_keyword, "from", "bar", quoted, "and", "baz") SELECT unquoted1, unquoted2, unquoted3, "create", "table", "select", "quoted1", "quoted2", "quoted3" FROM __temp__mytable',
             'DROP TABLE __temp__mytable',
         );
-	}
+    }
 
     /**
      * {@inheritdoc}
@@ -537,7 +546,13 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
         return array(
             'CREATE TEMPORARY TABLE __temp__mytable AS SELECT unquoted1, unquoted2, unquoted3, "create", "table", "select" FROM mytable',
             'DROP TABLE mytable',
-            'CREATE TABLE mytable (unquoted1 VARCHAR(255) NOT NULL, unquoted2 VARCHAR(255) NOT NULL, unquoted3 VARCHAR(255) NOT NULL, "create" VARCHAR(255) NOT NULL, "table" VARCHAR(255) NOT NULL, "select" VARCHAR(255) NOT NULL)',
+            'CREATE TABLE mytable (unquoted1 VARCHAR(255) NOT NULL --Unquoted 1
+, unquoted2 VARCHAR(255) NOT NULL --Unquoted 2
+, unquoted3 VARCHAR(255) NOT NULL --Unquoted 3
+, "create" VARCHAR(255) NOT NULL --Reserved keyword 1
+, "table" VARCHAR(255) NOT NULL --Reserved keyword 2
+, "select" VARCHAR(255) NOT NULL --Reserved keyword 3
+)',
             'INSERT INTO mytable (unquoted1, unquoted2, unquoted3, "create", "table", "select") SELECT unquoted1, unquoted2, unquoted3, "create", "table", "select" FROM __temp__mytable',
             'DROP TABLE __temp__mytable',
         );
@@ -581,7 +596,8 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
         return array(
             'CREATE TEMPORARY TABLE __temp__foo AS SELECT bar FROM foo',
             'DROP TABLE foo',
-            'CREATE TABLE foo (baz INTEGER DEFAULT 666 NOT NULL)',
+            'CREATE TABLE foo (baz INTEGER DEFAULT 666 NOT NULL --rename test
+)',
             'INSERT INTO foo (baz) SELECT bar FROM __temp__foo',
             'DROP TABLE __temp__foo',
         );
@@ -621,6 +637,26 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
         );
     }
 
+    protected function getInlineColumnCommentDelimiter()
+    {
+        return "\n";
+    }
+
+    protected function getInlineColumnRegularCommentSQL()
+    {
+        return "--Regular comment\n";
+    }
+
+    protected function getInlineColumnCommentRequiringEscapingSQL()
+    {
+        return "--Using inline comment delimiter \n-- works\n";
+    }
+
+    protected function getInlineColumnEmptyCommentSQL()
+    {
+        return "--\n";
+    }
+
     /**
      * {@inheritdoc}
      */