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.

Sign up for GitHub

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

Jump to bottom

Add support for MySQL auto_increment offset #950

Merged
merged 3 commits into from
Aug 21, 2023
Merged

Add support for MySQL auto_increment offset #950

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51c1b21
Added support for mysql auto_increment offset, should also fix issue …
ehoeve Aug 17, 2023
9368e98
cargo fmt
ehoeve Aug 17, 2023
75cea6b
Rename autoincrement to auto_increment to more closely follow convention
ehoeve Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/ast/helpers/stmt_create_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub struct CreateTableBuilder {
pub clone: Option<ObjectName>,
pub engine: Option<String>,
pub comment: Option<String>,
pub autoincrement_offset: Option<u32>,
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any reason to use a different style than the original auto_increment? I wonder if you considered:

Suggested change
pub autoincrement_offset: Option<u32>,
pub auto_increment_offset: Option<u32>,

?

If so can you explain why you chose this way?

I was thinking that following the exisiting convention would make the relevant code / field easier to search for (aka you can search for auto_increment in the code to see how auto_increment in SQL would be parsed.

Copy link
Contributor Author

@ehoeve ehoeve Aug 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@alamb Thanks for the feedback.
No specific reason, your suggestion of more closely following convention is a better option I think, so I updated the PR

pub default_charset: Option<String>,
pub collation: Option<String>,
pub on_commit: Option<OnCommit>,
Expand Down Expand Up @@ -98,6 +99,7 @@ impl CreateTableBuilder {
clone: None,
engine: None,
comment: None,
autoincrement_offset: None,
default_charset: None,
collation: None,
on_commit: None,
Expand Down Expand Up @@ -204,6 +206,11 @@ impl CreateTableBuilder {
self
}

pub fn autoincrement_offset(mut self, offset: Option<u32>) -> Self {
self.autoincrement_offset = offset;
self
}

pub fn default_charset(mut self, default_charset: Option<String>) -> Self {
self.default_charset = default_charset;
self
Expand Down Expand Up @@ -257,6 +264,7 @@ impl CreateTableBuilder {
clone: self.clone,
engine: self.engine,
comment: self.comment,
autoincrement_offset: self.autoincrement_offset,
default_charset: self.default_charset,
collation: self.collation,
on_commit: self.on_commit,
Expand Down Expand Up @@ -296,6 +304,7 @@ impl TryFrom<Statement> for CreateTableBuilder {
clone,
engine,
comment,
autoincrement_offset,
default_charset,
collation,
on_commit,
Expand Down Expand Up @@ -324,6 +333,7 @@ impl TryFrom<Statement> for CreateTableBuilder {
clone,
engine,
comment,
autoincrement_offset,
default_charset,
collation,
on_commit,
Expand Down
5 changes: 5 additions & 0 deletions src/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,7 @@ pub enum Statement {
clone: Option<ObjectName>,
engine: Option<String>,
comment: Option<String>,
autoincrement_offset: Option<u32>,
default_charset: Option<String>,
collation: Option<String>,
on_commit: Option<OnCommit>,
Expand Down Expand Up @@ -2263,6 +2264,7 @@ impl fmt::Display for Statement {
default_charset,
engine,
comment,
autoincrement_offset,
collation,
on_commit,
on_cluster,
Expand Down Expand Up @@ -2417,6 +2419,9 @@ impl fmt::Display for Statement {
if let Some(comment) = comment {
write!(f, " COMMENT '{comment}'")?;
}
if let Some(autoincrement_offset) = autoincrement_offset {
write!(f, " AUTO_INCREMENT {autoincrement_offset}")?;
}
if let Some(order_by) = order_by {
write!(f, " ORDER BY ({})", display_comma_separated(order_by))?;
}
Expand Down
12 changes: 12 additions & 0 deletions src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3550,6 +3550,17 @@ impl<'a> Parser<'a> {
None
};

let autoincrement_offset = if self.parse_keyword(Keyword::AUTO_INCREMENT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

https://dev.mysql.com/doc/refman/8.0/en/create-table.html

I got super confused for a moment because AUTO_INCREMENT can occur either as part of the table options (as this PR correctly does)


table_options:
    table_option [[,] table_option] ...

table_option: {
    AUTOEXTEND_SIZE [=] value
  | AUTO_INCREMENT [=] value
  | AVG_ROW_LENGTH [=] value
...

Or also as part of the column definition


column_definition: {
    data_type [NOT NULL | NULL] [DEFAULT {literal | (expr)} ]
      [VISIBLE | INVISIBLE]
      [AUTO_INCREMENT] [UNIQUE [KEY]] [[PRIMARY] KEY]
...

Which SQLParser already handles
https://github.com/sqlparser-rs/sqlparser-rs/blob/41e47cc0136c705a3335b1504d50ae8b22711b86/src/ast/ddl.rs#L587

let _ = self.consume_token(&Token::Eq);
let next_token = self.next_token();
match next_token.token {
Token::Number(s, _) => Some(s.parse::<u32>().expect("literal int")),
_ => self.expected("literal int", next_token)?,
}
} else {
None
};

let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) {
if self.consume_token(&Token::LParen) {
let columns = if self.peek_token() != Token::RParen {
Expand Down Expand Up @@ -3631,6 +3642,7 @@ impl<'a> Parser<'a> {
.clone_clause(clone)
.engine(engine)
.comment(comment)
.autoincrement_offset(autoincrement_offset)
.order_by(order_by)
.default_charset(default_charset)
.collation(collation)
Expand Down
25 changes: 25 additions & 0 deletions tests/sqlparser_mysql.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,31 @@ fn parse_create_table_comment() {
}
}

#[test]
fn parse_create_table_autoincrement_offset() {
let canonical =
"CREATE TABLE foo (bar INT NOT NULL AUTO_INCREMENT) ENGINE=InnoDB AUTO_INCREMENT 123";
let with_equal =
"CREATE TABLE foo (bar INT NOT NULL AUTO_INCREMENT) ENGINE=InnoDB AUTO_INCREMENT=123";

for sql in [canonical, with_equal] {
match mysql().one_statement_parses_to(sql, canonical) {
Statement::CreateTable {
name,
autoincrement_offset,
..
} => {
assert_eq!(name.to_string(), "foo");
assert_eq!(
autoincrement_offset.expect("Should exist").to_string(),
"123"
);
}
_ => unreachable!(),
}
}
}

#[test]
fn parse_create_table_set_enum() {
let sql = "CREATE TABLE foo (bar SET('a', 'b'), baz ENUM('a', 'b'))";
Expand Down
Loading