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

Fix two regexes related to SQL create statement parsing #140

Merged
merged 4 commits into from
Apr 21, 2023

Conversation

d-Rickyy-b
Copy link
Contributor

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Fixes two regex issues that prevented the proper parsing of SQL create statements. Fixes #139. For a more detailed description, please check the linked issue.

@d-Rickyy-b
Copy link
Contributor Author

d-Rickyy-b commented Mar 25, 2023

Regex 1 - Matching full SQL create statement:

Regex 2 - Renaming table to __temp:

@jinzhu
Copy link
Member

jinzhu commented Apr 11, 2023

Hello @d-Rickyy-b

Thank you for your PR, can you help to add more tests?

Thank you.

@d-Rickyy-b
Copy link
Contributor Author

Hi @jinzhu, I added some tests, although the code in the migrator file is not easily testable.

Btw. could you explain to me, why the sqliteSeparator contains \t? Is there any valid reason to it? I couldn't find any explanation by checking git blame.

sqliteSeparator = "`|\"|'|\t"

This \t is part of another parsing problem related to (multiple) tab characters being part the DDL.
Also there are quite some potential issues related to whitespaces everywhere where you check for a literal space character followed by keywords:

if strings.Contains(matchUpper, " NOT NULL") {

} else if strings.Contains(matchUpper, " NULL") {

if strings.Contains(matchUpper, " UNIQUE") {

if strings.Contains(matchUpper, " PRIMARY") {

These probably should be replaced with regexes and \s+ instead of literal spaces.

@jinzhu jinzhu merged commit 5acf810 into go-gorm:master Apr 21, 2023
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.

tableRegexp does not match certain multi-line "CREATE TABLE" statements
2 participants