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 … RENAME COLUMN … TO … not understood by the parser/linter #430

Closed
niconoe- opened this issue Mar 13, 2023 · 1 comment · Fixed by #432
Closed

ALTER TABLE … RENAME COLUMN … TO … not understood by the parser/linter #430

niconoe- opened this issue Mar 13, 2023 · 1 comment · Fixed by #432
Labels

Comments

@niconoe-
Copy link
Contributor

niconoe- commented Mar 13, 2023

When trying to parse the following SQL statement, the AlterStatement createed regarding this SQL is missing the "TO" option and the new name of the column.

ALTER TABLE potato RENAME COLUMN `KIND` TO `VARIETY`;

The linter give also the error:

> bin/lint-query --query 'ALTER TABLE potato RENAME COLUMN `KIND` TO `VARIETY`;'
#1: Virgule manquante avant le début d’une nouvelle opération ALTER. (near "TO" at position 40)

(French translation for "#1: Missing comma before the beginning of a new ALTER operation.").

@williamdes williamdes added the bug label Mar 13, 2023
@niconoe-
Copy link
Contributor Author

After a little analysis, this is because the AlterOperation is expected to be templated like this:
[...OPTIONS] <field> [...UNKNOWN]

But this is not true for every ALTER operation, even when staying in the context of a table. Or we can make it true, but that means the [...UNKNOWN] part is never interpreted correctly, as it just contains a list of tokens without any useful information.

I was about to provide a PR so that only for RENAME COLUMN operation, we parse again the options to get the TO and to merge it, as to me, the AlterOperation would be cleaner this way, but this breaks how the Builder is behaving. I could also add a specific case for the RENAME COLUMN in the builder, but it looks like putting a tape on a hole rather than really parsing the alter operations correctly.

Anyway, I'll provide my PR and if you can find a cleaner way to manage this situation, I'll be your man 😆

williamdes added a commit that referenced this issue Mar 13, 2023
Pull-request: #432

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
2 participants