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

Signed int parse #8189

Merged
merged 4 commits into from
May 27, 2021
Merged

Signed int parse #8189

merged 4 commits into from
May 27, 2021

Conversation

GuptaManan100
Copy link
Member

Description

The following query does not work correctly in the parser

alter table t add column iii int signed not null;

This PR fixes it by adding the optional SIGNED token after the numeric type names.

Related Issue(s)

Fixes #8153

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

GuptaManan100 and others added 2 commits May 25, 2021 18:31
Signed-off-by: GuptaManan100 <manan@planetscale.com>
Signed-off-by: GuptaManan100 <manan@planetscale.com>
Co-authored-by: frouioui <florent.poinsard@outlook.fr>
@shlomi-noach
Copy link
Contributor

Does this need support in ast_print.go?

Copy link
Collaborator

@systay systay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not drop the signed/unsigned part in the AST

Signed-off-by: GuptaManan100 <manan@planetscale.com>
Signed-off-by: GuptaManan100 <manan@planetscale.com>
@GuptaManan100 GuptaManan100 requested a review from systay May 27, 2021 07:38
@GuptaManan100
Copy link
Member Author

We are already storing the signed/unsigned part in the AST. Its just that signed is the default so it is dropped. Since we already had the functionality of storing this in AST, we do not require any changes to ast_format.go. I have added a test which checks that we indeed do not ignore unsigned.

Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

@systay systay merged commit 8aaeed1 into vitessio:master May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error parsing DDL: int signed
3 participants