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

Misleading error message for typo in IF NOT EXISTS #1169

Open
tisonkun opened this issue Mar 10, 2024 · 7 comments
Open

Misleading error message for typo in IF NOT EXISTS #1169

tisonkun opened this issue Mar 10, 2024 · 7 comments

Comments

@tisonkun
Copy link
Contributor

tisonkun commented Mar 10, 2024

See also GreptimeTeam/greptimedb#3471.

For sqlparser-rs -

fn main() {
    let sql = "CREATE TABLE IF NOT EXIST ();";

    let dialect = GenericDialect {};

    let ast = Parser::parse_sql(&dialect, sql).unwrap();

    println!("AST: {ast:?}");
}

throws:

called `Result::unwrap()` on an `Err` value: ParserError("Expected end of statement, found: NOT at Line: 1, Column 17")

IIUC if we found IF already, the next keywords SHOULD BE NOT EXIST and thus the parsing logics should be .expect_keywords.

@tisonkun
Copy link
Contributor Author

Some corner case -

  • IF as the table name can be legal.
  • What CREATE TABLE NOT EXISTS ... should report?

@alamb alamb added help wanted Extra attention is needed and removed help wanted Extra attention is needed labels Mar 11, 2024
@tisonkun

This comment was marked as outdated.

@tisonkun

This comment was marked as outdated.

@alamb
Copy link
Collaborator

alamb commented Mar 11, 2024

👍 makes sense to me -- a nicer error message would indeed be good

@tisonkun
Copy link
Contributor Author

@alamb the challenging part is described in #1169 (comment). Even if we look ahead IF, we cannot simply expect_keywords because it can be a table identifier.

Keep a backtrace would be the way but it seems too complex for me to working out now ...

@alamb
Copy link
Collaborator

alamb commented Mar 11, 2024

You could potentially special case the error message, perhaps

@tisonkun
Copy link
Contributor Author

I made an attempt at GreptimeDB - GreptimeTeam/greptimedb#3817

If things go well, I'll try to make similar changes to sqlparser-rs.

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

No branches or pull requests

2 participants