Skip to content

Require space after -- to start single line comment in MySQL #1705

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

Merged
merged 3 commits into from
Feb 5, 2025
Merged

Require space after -- to start single line comment in MySQL #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9eba94c
Require space after -- to start single line comment in MySQL
hansott Feb 4, 2025
2b4b6ce
Refactor and improve tests
hansott Feb 4, 2025
4a90098
Feedback
hansott Feb 5, 2025
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
9 changes: 9 additions & 0 deletions src/dialect/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,15 @@ pub trait Dialect: Debug + Any {
fn supports_table_hints(&self) -> bool {
false
}

/// Returns true if this dialect requires a whitespace character after `--` to start a single line comment.
///
/// MySQL: <https://dev.mysql.com/doc/refman/8.4/en/ansi-diff-comments.html>
/// e.g. UPDATE account SET balance=balance--1
// WHERE account_id=5752 ^^^ will be interpreted as two minus signs instead of a comment
fn requires_single_line_comment_whitespace(&self) -> bool {
false
}
}

/// This represents the operators for which precedence must be defined
Expand Down
4 changes: 4 additions & 0 deletions src/dialect/mysql.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ impl Dialect for MySqlDialect {
fn supports_table_hints(&self) -> bool {
true
}

fn requires_single_line_comment_whitespace(&self) -> bool {
true
}
}

/// `LOCK TABLES`
Expand Down
105 changes: 99 additions & 6 deletions src/tokenizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1229,14 +1229,26 @@ impl<'a> Tokenizer<'a> {
// operators
'-' => {
chars.next(); // consume the '-'

match chars.peek() {
Some('-') => {
chars.next(); // consume the second '-', starting a single-line comment
let comment = self.tokenize_single_line_comment(chars);
Ok(Some(Token::Whitespace(Whitespace::SingleLineComment {
prefix: "--".to_owned(),
comment,
})))
let mut is_comment = true;
if self.dialect.requires_single_line_comment_whitespace() {
is_comment = Some(' ') == chars.peekable.clone().nth(1);
Copy link

Choose a reason for hiding this comment

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

Usually, -- is used independently to separate segments of SQL statements. For example:

BEGIN;
--
-- Add field field_1 to table1
--
ALTER TABLE `table1` ADD COLUMN `field_1` datetime(6) NULL;

--
-- Add field field_2 to table2
--
ALTER TABLE `table2` ADD COLUMN `field_2` datetime(6) NULL;

COMMIT;

In this case, -- will not be recognized as a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right!

It was still on my planning to create a follow-up PR... I was doing more research but then the PR got merged

Copy link
Contributor

Choose a reason for hiding this comment

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

A follow on PR would be most appreciated

}

if is_comment {
chars.next(); // consume second '-'
let comment = self.tokenize_single_line_comment(chars);
return Ok(Some(Token::Whitespace(
Whitespace::SingleLineComment {
prefix: "--".to_owned(),
comment,
},
)));
}

self.start_binop(chars, "-", Token::Minus)
}
Some('>') => {
chars.next();
Expand Down Expand Up @@ -3685,4 +3697,85 @@ mod tests {
],
);
}

#[test]
fn test_whitespace_required_after_single_line_comment() {
all_dialects_where(|dialect| dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT --'abc'",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Minus,
Token::Minus,
Token::SingleQuotedString("abc".to_string()),
],
);

all_dialects_where(|dialect| dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT -- 'abc'",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Whitespace(Whitespace::SingleLineComment {
prefix: "--".to_string(),
comment: " 'abc'".to_string(),
}),
],
);

all_dialects_where(|dialect| dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT --",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Minus,
Token::Minus,
],
);
}

#[test]
fn test_whitespace_not_required_after_single_line_comment() {
all_dialects_where(|dialect| !dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT --'abc'",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Whitespace(Whitespace::SingleLineComment {
prefix: "--".to_string(),
comment: "'abc'".to_string(),
}),
],
);

all_dialects_where(|dialect| !dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT -- 'abc'",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Whitespace(Whitespace::SingleLineComment {
prefix: "--".to_string(),
comment: " 'abc'".to_string(),
}),
],
);

all_dialects_where(|dialect| !dialect.requires_single_line_comment_whitespace())
.tokenizes_to(
"SELECT --",
vec![
Token::make_keyword("SELECT"),
Token::Whitespace(Whitespace::Space),
Token::Whitespace(Whitespace::SingleLineComment {
prefix: "--".to_string(),
comment: "".to_string(),
}),
],
);
}
}
15 changes: 15 additions & 0 deletions tests/sqlparser_mysql.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3244,3 +3244,18 @@ fn parse_double_precision() {
"CREATE TABLE foo (bar DOUBLE(11,0))",
);
}

#[test]
fn parse_looks_like_single_line_comment() {
mysql().one_statement_parses_to(
"UPDATE account SET balance=balance--1 WHERE account_id=5752",
"UPDATE account SET balance = balance - -1 WHERE account_id = 5752",
);
mysql().one_statement_parses_to(
r#"
UPDATE account SET balance=balance-- 1
WHERE account_id=5752
"#,
"UPDATE account SET balance = balance WHERE account_id = 5752",
);
}