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

Allow parsing of mysql empty row inserts #783

Merged
merged 1 commit into from Jan 2, 2023

Conversation

Jefffrey
Copy link
Contributor

@Jefffrey Jefffrey commented Jan 1, 2023

Closes #624

Allows parsing of

INSERT INTO tb () VALUES ();

for MySql dialect

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3816502109

  • 61 of 61 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 86.06%

Totals Coverage Status
Change from base Build 3801061789: 0.03%
Covered Lines: 13131
Relevant Lines: 15258

💛 - Coveralls

Copy link
Collaborator

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks good to me - thanks @Jefffrey

I actually think we could allow the empty values list for all dialects (not just MySQL) per the https://github.com/sqlparser-rs/sqlparser-rs#syntax-vs-semantics section

That would keep this PR smaller and the sqlparser-rs code simpler

What do you think?

let cols = self.parse_comma_separated(Parser::parse_identifier)?;
self.expect_token(&Token::RParen)?;
Ok(cols)
if allow_empty && self.peek_token().token == Token::RParen {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@Jefffrey
Copy link
Contributor Author

Jefffrey commented Jan 1, 2023

One thing I noticed however, was for postgresql it would be a parser exception. For example, when running:

insert into tb () values ();

Get exception

Caused by: org.postgresql.util.PSQLException: ERROR: syntax error at or near ")"
  Position: 17

Similarly, for the same in MSSql:

Caused by: com.microsoft.sqlserver.jdbc.SQLServerException: Incorrect syntax near ')'.

So seems to suggest that for these dialects, it shouldn't parse at all. Thus, I'm a bit hesitant on enabling for all dialects. Not to mention, that the methods modified (parse_values() and parse_parenthesized_column_list()) are used in more places than just for parsing INSERT INTO statements (e.g. I believe parse_parenthesized_column_list() is used for parsing EXCEPT () for bigquery which requires it to be non-empty:

assert_eq!(
bigquery_and_generic()
.parse_sql_statements("SELECT * EXCEPT () FROM employee_table")
.unwrap_err()
.to_string(),
"sql parser error: Expected identifier, found: )"
);

So those methods would still likely require the new allow_empty parameter so code might not be too simplified even if enabling the empty row insert for all dialects (unless refactor to a separate method specifically for INSERT INTO () tb VALUES () statements, if possible)

@alamb

@alamb alamb merged commit 98403c0 into sqlparser-rs:main Jan 2, 2023
@Jefffrey Jefffrey deleted the 624_mysql_empty_row_insert branch January 2, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support MySQL's empty row insert
3 participants