Improve tests on reserved keyword

[homersimpsons]

Q	A
Type	improvement
Fixed issues	N/A (extract of #6765)

Summary

• Use select as reserved keyword (user does not seem to be reserved on sqlite)
• Ensure comments are retrieved correctly

[homersimpsons]

Looks like there is an issue with SQLite Table comment (https://github.com/doctrine/dbal/actions/runs/14295646317/job/40062871091).

I dug a bit and it looks like it should be added in

dbal/src/Platforms/SQLitePlatform.php

Lines 293 to 298 in c4ae845

	$tableComment = '';
	if (isset($parameters['comment'])) {
	$comment = trim($parameters['comment'], " '");
	$tableComment = $this->getInlineTableCommentSQL($comment);
	}

But somehow the 'comment' option is not set there. It does not exist either in $table->getOption('comment'):

dbal/src/Platforms/AbstractPlatform.php

Line 867 in c4ae845

$sql[] = $this->getCommentOnTableSQL($tableName->toSQL($this), $table->getOption('comment'));

(Here we do not get inside anyway as $this->supportsCommentOnStatement() is false (default) for sqlite)

For the record, it works if I create and read the commentas manually:

$connection->executeStatement(<<<'SQL'
CREATE TABLE "user" --table comment
(
    "user" INT --column comment
)
SQL
);

$schemaManager = $connection->createSchemaManager();
$table = schemaManager->introspectTable('"user"');
var_dump($table->getComment()); // "table comment"
var_dump($table->getColumn('user')->getComment()); // "column comment"

[morozov]

@homersimpsons I recently added a similar change to another test (see #6893).

As we removed reserved keyword lists from 5.0 (see #6589), there isn't any difference in handling the names that are reserved keywords (e.g. "select") and any other quoted identifiers (e.g. the ones that contain letters in the upper and lower case).

As for why the updated test is failing ,

But somehow the 'comment' option is not set there. It does not exist either in $table->getOption('comment')

I don't see this option being set on the table in your updated test.

[homersimpsons]

@homersimpsons I recently added a similar change to another test (see #6893).

As we removed reserved keyword lists from 5.0 (see #6589), there isn't any difference in handling the names that are reserved keywords (e.g. "select") and any other quoted identifiers (e.g. the ones that contain letters in the upper and lower case).

In fact your added test should be enough.

As for why the updated test is failing ,

But somehow the 'comment' option is not set there. It does not exist either in $table->getOption('comment')

I don't see this option being set on the table in your updated test.

Oh in fact, sorry, I updated the test, but when I run them locally on sqlite I have table comment instead of column comment for the column. The result from getCreateTableSQL is correct, I believe the issue is because in the test the table and the column have the same name.

I can fix this with a slight modification of the parseColumnCommentFromSQL regex:

-{[\s(,](?:\Wselect\W|\W"select"\W)(?:\([^)]*?\)|[^,(])*?,?((?:(?!\n))(?:\s*--[^\n]*\n?)+)}i
+{[\s(,](?:\Wselect\W|\W"select"\W)\s(?:\([^)]*?\)|[^,(])*?,?((?:(?!\n))(?:\s+--[^\n]*\n?)+)}i
                                   ^^ require a space                        ^ require at least a space

But I do not know if this fix is correct, i.e. how constrained is the create table output in sqlite. From my understanding it just returns the exact CREATE TABLE we used. When using ALTER TABLE it somehow removes the correct comment (looks like we cannot add any comment using ALTER TABLE):

CREATE table "test" (/* abc */ "id" INTEGER, "test" INTEGER --abc
);
SELECT sql FROM sqlite_master WHERE type = 'table' and name = 'test'; 
/*
CREATE TABLE "test" (/* abc */ "id" INTEGER, "test" INTEGER --abc
)
*/

ALTER TABLE "test" ADD COLUMN "foo" INTEGER; --no comment
SELECT sql FROM sqlite_master WHERE type = 'table' and name = 'test';
/*
CREATE TABLE "test" (/* abc */ "id" INTEGER, "test" INTEGER --abc
, "foo" INTEGER)
*/

ALTER TABLE "test" DROP COLUMN "test";
SELECT sql FROM sqlite_master WHERE type = 'table' and name = 'test'; 
/*
CREATE TABLE "test" (/* abc */ "id" INTEGER, "foo" INTEGER)
*/

[morozov]

I believe the issue is because in the test the table and the column have the same name.

If that's an issue, I think it's worth being reproduced and tracked separately, not as part of this PR. As we agreed, there is an existing test that covers this scenario.

A few facts on the support for table/column comments on SQLite:

1. There's no native support for them, so whatever we implement will not work exactly as on other databases.
2. The approach of parsing DDL with a single regular expression is error-prone. The right way to do that would be use a parser (see Add ANTLR4-generated parser for SQLite #5694).
3. I don't know how practically relevant the functionality of adding comments to tables and columns is once Drop type comments entirely #5107 has been merged.