Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ALTER TABLE … MODIFY … ENUM('<reserved_keyword>') is being wrongly parsed. #478

Closed
niconoe- opened this issue Jun 9, 2023 · 7 comments · Fixed by #485
Closed

ALTER TABLE … MODIFY … ENUM('<reserved_keyword>') is being wrongly parsed. #478

niconoe- opened this issue Jun 9, 2023 · 7 comments · Fixed by #485
Assignees
Labels
Milestone

Comments

@niconoe-
Copy link
Contributor

niconoe- commented Jun 9, 2023

When trying to parse a statement like this:

ALTER TABLE `MY_TABLE` 
MODIFY `FOO` INT(11) NULL,
MODIFY `MY_COLUMN` ENUM('INSERT','UPDATE','DELETE','REPLACE') NULL,
MODIFY `BAR` VARCHAR(255) NULL;

SQL-Parser fails because it considers each value of the enum as a real keyword, instead of a simple string. Hence, next operations on the ALTER TABLE statements are not understood correctly, and it makes fail the parser.
This causes:

#n: A new statement was found, but no delimiter between it and the previous one. (near "&#039;INSERT&#039;" at position xxx)

Thanks 🙂

@williamdes
Copy link
Member

Could be related to #450 and it's fix

@niconoe-
Copy link
Contributor Author

niconoe- commented Jun 9, 2023

This occurs on the latest version of sql-parser (5.8.0 as of today), so either this is not fixed, not related to #450, nor the fix is not yet released 🙂

I'm not sure this is related TBH, as OP of #450 talked about missing commas, while my issue here is really about the parser (or the lexer?) not being aware that the strings in the ENUM are actually not reserved symbols or keywords for statements.

@Tithugues
Copy link
Contributor

Could be related to #450 and it's fix

Did I break something? 😅

@williamdes
Copy link
Member

Well, more tests are needed to know for sure ^^

@niconoe-
Copy link
Contributor Author

Some tests are added in #485 's 1st commit to show that using values in the ENUM like "TABLE" or "INSERT" are generating a parsing error.
The 2nd commit fixes that unwanted behavior and shows that the parser no longer have any error.

@williamdes williamdes added the bug label Jun 21, 2023
@niconoe-
Copy link
Contributor Author

Ok, wait a minute, I screwed up something.

I just forgot to add the unit tests dataset I created 😅 . After actually adding them, the fix doesn't work.

That is because we are entering the if(! empty(Parser::$statementParsers[$token->value])) when doing the "SET", but not the "ENUM", and that is because "SET" is also a statement keyword, while "ENUM" is not…

We are just "lucky" to enter in this "if" block when using "SET" as a type, making no difference between "SET" as a type, and "SET" as a statement.

I'll fix that in a moment.

@niconoe-
Copy link
Contributor Author

Ok, wait a minute, I screwed up something.

I just forgot to add the unit tests dataset I created sweat_smile . After actually adding them, the fix doesn't work.

That is because we are entering the if(! empty(Parser::$statementParsers[$token->value])) when doing the "SET", but not the "ENUM", and that is because "SET" is also a statement keyword, while "ENUM" is not…

We are just "lucky" to enter in this "if" block when using "SET" as a type, making no difference between "SET" as a type, and "SET" as a statement.

I'll fix that in a moment.

OK, that is fixed now. One of the side effect is that the tokens describing the ENUM are no longer stored in the unknown field, just like it was the case for the SET. So at least, both keywords are now producing the same behavior.

If that's an issue to not have the tokens in this case, I would suggest to create another issue IMO, as this also affected the SET type.

@williamdes williamdes self-assigned this Jun 29, 2023
@williamdes williamdes added this to the 5.9.0 milestone Jun 29, 2023
williamdes added a commit that referenced this issue Jun 29, 2023
Pull-request: #485

Signed-off-by: William Desportes <williamdes@wdes.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants