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

Grammar fixes for customer issue using non-reserved keywords in insert into statements #189

Merged
merged 6 commits into from
Sep 15, 2022

Conversation

fulghum
Copy link

@fulghum fulghum commented Sep 15, 2022

A customer reported an issue on Discord with using insert into t(pk, comment) values (1, 1). Comment is a keyword, but not a reserved keyword, so this is valid syntax, but our parser was failing on it.

A few changes to tidy up:

  • Updated tests to trigger the customer reported parsing bug when an (unquoted) keyword is used in a list of columns in an insert into statement.
  • renamed column_name_safe_reserved_keyword to column_name_safe_keyword, since the contents are keywords, but are not "reserved" keywords that need to be backtick quoted
  • fixed a bug with recursion for insert into column names (ins_column_list rule)
  • removed avg,count,sum,min,max from lists of reserved keywords, since they are not reserved in MySQL and can parse correctly in tests without being backtick quoted.
  • added some more comments to try and make the intent of different grammar rules clearer.

…quoted) keyword is used in a list of columns in an insert into statement.
- renaming column_name_safe_reserved_keyword to column_name_safe_keyword, since the contents are keywords, not "reserved" keywords that need to be backtick quoted
- fixing a bug with recursion for insert into column names (ins_column_list rule)
- removing avg,count,sum,min,max from lists of reserved keywords, since they are not reserved in MySQL and can parse correctly in tests.
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM!

iTest := "INSERT INTO t (%s) VALUES (1)"
iTest1 := "INSERT INTO t_t (%s) VALUES ('one')"
iTest2 := "INSERT INTO t_t (pk, %s) VALUES (1, 'one')"
iTest3 := "INSERT INTO t_t (pk, %s, c1) VALUES ('one', 1)"
Copy link
Member

Choose a reason for hiding this comment

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

Might change this to have the right number of values

@fulghum fulghum merged commit 9b0a34e into main Sep 15, 2022
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.

2 participants