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 SQLite message about incorrect double quotes #1334

Merged

Conversation

fvaillancourt
Copy link

When migrating a SQLite DB I was getting the following warning out of SQLite
Message: double-quoted string literal: "VersionInfo"
Error code: 28

Frédéric Vaillancourt added 2 commits September 8, 2020 13:44
Message: double-quoted string literal: "VersionInfo"
Error code: 28
@fvaillancourt
Copy link
Author

Sorry about single quote, show have run tests

@jzabroski
Copy link
Collaborator

@fvaillancourt I think the SqliteProcesssor should use the SqliteQuoter.

  1. Update SqliteProcessorFactory to inject the same instance of the quoter in the generator and the processor, here: https://github.com/fluentmigrator/fluentmigrator/blob/e82aafa20e6dbe3cefa221303fe23cf8bf59fffd/src/FluentMigrator.Runner.SQLite/Processors/SQLite/SQLiteProcessorFactory.cs
  2. Update SqliteProcessor to take a SqliteQuoter
  3. Remove the string.Replace nastiness with the SqliteQuoter
  4. Ensure tests pass
  5. Maybe run some integration tests locally to ensure end-to-end tests work for your scenario (no more warnings to the console)

@fvaillancourt
Copy link
Author

@jzabroski How about that? I think it is close to what you had in mind? I did test it in our app and it does remove the warning SQLite was outputing.

@jzabroski
Copy link
Collaborator

@fvaillancourt yes, perfect. Thank you so much! Love when people quickly follow up on reviews. Puts a pep in my step.

@jzabroski jzabroski merged commit a8e960c into fluentmigrator:master Sep 9, 2020
@jzabroski jzabroski self-requested a review September 9, 2020 20:38
@jzabroski jzabroski added this to the 3.2.10 milestone Sep 9, 2020
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.

None yet

2 participants