Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/ast/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2361,6 +2361,8 @@ pub struct CreateIndex {
pub name: Option<ObjectName>,
#[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]
pub table_name: ObjectName,
/// Index type used in the statement. Can also be found inside [`CreateIndex::index_options`]
/// depending on the position of the option within the statement.
pub using: Option<IndexType>,
pub columns: Vec<IndexColumn>,
pub unique: bool,
Expand Down
15 changes: 10 additions & 5 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7061,19 +7061,24 @@ impl<'a> Parser<'a> {
pub fn parse_create_index(&mut self, unique: bool) -> Result<Statement, ParserError> {
let concurrently = self.parse_keyword(Keyword::CONCURRENTLY);
let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);

let mut using = None;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a fan of this mut but also not a fan of returning a tuple of (index_name, using) at line 7067.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems okay IMO.


let index_name = if if_not_exists || !self.parse_keyword(Keyword::ON) {
let index_name = self.parse_object_name(false)?;
// MySQL allows `USING index_type` either before or after `ON table_name`
using = self.parse_optional_using_then_index_type()?;
self.expect_keyword_is(Keyword::ON)?;
Some(index_name)
} else {
None
};

let table_name = self.parse_object_name(false)?;
let using = if self.parse_keyword(Keyword::USING) {
Some(self.parse_index_type()?)
} else {
None
};

// MySQL allows having two `USING` clauses.
// In that case, the second clause overwrites the first.
using = self.parse_optional_using_then_index_type()?.or(using);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be something like

using = using.or_else(|| self.parse_optional_using_then_index_type()).transpose()?;

in that we should only look to parse again if we didn't parse a value already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As Michael pointed out earlier, MySQL does allow having two "USING" clauses. In that case, the second one overwrites the first.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth a comment on this line, because at first glance (or if you haven't tried it on MySQL yourself) it does seem backwards 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, double checking do the tests include the scenario where two USING clauses are specified? If not we can probably add those

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Added a second test with two USING clauses. Force-pushed to resolve conflicts and squashed the commits.


let columns = self.parse_parenthesized_index_column_list()?;

Expand Down
43 changes: 43 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17246,3 +17246,46 @@ fn parse_invisible_column() {
_ => panic!("Unexpected statement {stmt}"),
}
}

#[test]
fn parse_create_index_different_using_positions() {
let sql = "CREATE INDEX idx_name USING BTREE ON table_name (col1)";
let expected = "CREATE INDEX idx_name ON table_name USING BTREE (col1)";
match all_dialects().one_statement_parses_to(sql, expected) {
Statement::CreateIndex(CreateIndex {
name,
table_name,
using,
columns,
unique,
..
}) => {
assert_eq!(name.unwrap().to_string(), "idx_name");
assert_eq!(table_name.to_string(), "table_name");
assert_eq!(using, Some(IndexType::BTree));
assert_eq!(columns.len(), 1);
assert!(!unique);
}
_ => unreachable!(),
}

let sql = "CREATE INDEX idx_name USING BTREE ON table_name (col1) USING HASH";
let expected = "CREATE INDEX idx_name ON table_name USING BTREE (col1) USING HASH";
match all_dialects().one_statement_parses_to(sql, expected) {
Statement::CreateIndex(CreateIndex {
name,
table_name,
columns,
index_options,
..
}) => {
assert_eq!(name.unwrap().to_string(), "idx_name");
assert_eq!(table_name.to_string(), "table_name");
assert_eq!(columns.len(), 1);
assert!(index_options
.iter()
.any(|o| o == &IndexOption::Using(IndexType::Hash)));
}
_ => unreachable!(),
}
}