CREATE TABLE statement not properly parsed or build

[DBA-HH]

$sql    = 'CREATE TABLE tab1 (`col1` TIMESTAMP /*!40100 DEFAULT NULL */)';
$parser = new Parser($sql);
$s3     = $parser->statements[0]->build();
// $s3 wrongly contains end-of-comment
// CREATE TABLE tab1 (
//  `col1` timestamp DEFAULT NULL */
//)

//PHP 7.1.18
//"name": "phpmyadmin/sql-parser",
//"version": "5.0.0",

[niconoe-]

I'm not promising anything, but I'll try to have a look on this one.

[williamdes]

But for me it it blocking because you can not re-use the built statement because of the */ but thank you for looking into it

[niconoe-]

After having a look at it, it is actually really hard if we continue using this logic for parsing. Each comments (even MYSQL conditionnal comments) are always excluded for every parsing step, while it shouldn't be if the comment is a MySQL conditionnal comment.

I have also an issue about what to expect as a content here. Regarding at the example, do we expect:

CREATE TABLE tab1 (
  `col1` timestamp /*!40100 DEFAULT NULL */
)

or

CREATE TABLE tab1 (
  `col1` timestamp DEFAULT NULL
)

because mysql_version used is >= 4.1.0, and

CREATE TABLE tab1 (
  `col1` timestamp
)

if mysql_version used is < 4.1.0

Maybe a way to fix this is to firstly parse all MySQL conditional comments and replace them by either a comment (so it will be automatically removed by the current process) or replace them by actual executed code (the token list is updated in that way today, so it would be effortless) depending on the current mysql_version used.

What you you think about?

[williamdes]

@niconoe- maybe we should keep mysql comments so the input = output and we do not change the behavior of the input given by the user

Would it be a solution?

[niconoe-]

If this is something we expect, then a PR I can provide is just that every parsers should not ignore comments if they are MySQL commands. And that way, it's an easy fix.

Actually, I'm not fluent enough in MySQL language to know if this is a correct behavior to let them or if they need to be interpreted. But I don't think we parse any query in a given version so maybe the parser is not able to interpret them. I agree that, if the Parser is not able to interpret those comment, we should keep them.

Is that your thoughts too? Thanks for your feedback.

[williamdes]

If this is something we expect, then a PR I can provide is just that every parsers should not ignore comments if they are MySQL commands. And that way, it's an easy fix.

Maybe we should try this option but be sure it does not break the data Only add them back

@devenbansod @ibennetch any thoughts on this issue?

[williamdes]

~~This is probably fixed by #438~~ Edit: it's not fixed, I added a test for it in 32e10ad522e4769d89178d3d5cbf69e7731ca6c1