Fix/sqlite column comment parsing

[homersimpsons]

Q	A
Type	bug
Fixed issues

Summary

Issue found in #6897 (comment)

When a table and its column share the same name and they both have a comment DBAL will return the table comment instead of the column comment.

Example:

CREATE TABLE "user" --table comment
(
    "user" INT --column comment
)

From the above link, here is the change:

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

Note that I made the fix for DBAL 5.0 but the issue also exists in 4.2: This now targets 4.2

dbal/src/Schema/SQLiteSchemaManager.php

Line 393 in 5bbe026

. '(?:\([^)]*?\)|[^,(])*?,?((?:(?!\n))(?:\s*--[^\n]*\n?)+)}i';

[morozov]

Note that I made the fix for DBAL 5.0 but the issue also exists in 4.2

Could you try retargeting it against 4.2.x then? I don't think this code has changed significantly between these versions.

[homersimpsons]

Note that I made the fix for DBAL 5.0 but the issue also exists in 4.2

Could you try retargeting it against 4.2.x then? I don't think this code has changed significantly between these versions.

Done

I'm also asking myself if regex is the best approach vs parsing the statement.

[morozov]

As I understand, this patch requires a space right after the table name in a CREATE TABLE statement. But SQLite doesn't require it.

The following is a valid SQLite query:

CREATE TABLE "user"--table comment
(
    "user" INT--column comment
)

Note that GitHub's code colorizer parses it correctly.

[morozov]

I'm also asking myself if regex is the best approach vs parsing the statement.

Parsing is definitely better. A simple regular expression cannot accommodate the entire SQL syntax, so it will be prone to errors in edge cases.

I tried using a parser some time ago (see #5694). It worked but it didn't justify bringing in a new dependency back then. I'm curious if we can find some common ground and write a simpler parser that wouldn't require external dependencies, would solve only a relevant subset of the problems, and which we could maintain ourselves.

UPD: looking at the syntax, I don't think writing a parser manually is easy and even feasible. Another alternative might be to generate a parser and then remove all unused code.