Query separator is insufficient

[straight-shoota]

Migration.from_io detects a trailing semicolon as the end of an SQL statement.

This can cause problems with queries with a trailing semicolon inside. This can be perfectly valid in multiple situations, such as inside a string literal.

For example, the following migration leads to an error.

-- drift:migrate
SELECT 'foo;
  bar';

-- drift:rollback

$ bin/drift migrate
Migrating: 20240506145234_test.sql
Unhandled exception: unrecognized token: "'foo;" (SQLite3::Exception)
  from lib/sqlite3/src/sqlite3/statement.cr:81:5 in 'check'
  from lib/sqlite3/src/sqlite3/statement.cr:4:5 in 'initialize'
  from lib/sqlite3/src/sqlite3/statement.cr:2:3 in 'new'
  from lib/sqlite3/src/sqlite3/connection.cr:63:5 in 'build_prepared_statement'
  from lib/db/src/db/connection.cr:60:42 in 'fetch_or_build_prepared_statement'
  from lib/db/src/db/session_methods.cr:25:16 in 'build'
  from lib/db/src/db/query_methods.cr:275:7 in 'exec'
  from src/drift/migration.cr:43:9 in 'run'
  from src/drift/migrator.cr:264:37 in 'apply_batch'
  from src/drift/migrator.cr:120:7 in 'apply!'
  from src/drift/commands/migrate.cr:33:13 in 'run'
  from src/drift/commands/command.cr:45:9 in 'run'
  from src/cli.cr:60:13 in '->'
  from /home/linuxbrew/.linuxbrew/Cellar/crystal/1.12.1_1/share/crystal/src/option_parser.cr:488:22 in 'parse'
  from src/cli.cr:77:7 in 'run'
  from src/cli.cr:27:7 in 'run'
  from src/cli.cr:26:5 in 'run'
  from src/cli.cr:83:3 in '__crystal_main'
  from /home/linuxbrew/.linuxbrew/Cellar/crystal/1.12.1_1/share/crystal/src/crystal/main.cr:129:5 in 'main_user_code'
  from /home/linuxbrew/.linuxbrew/Cellar/crystal/1.12.1_1/share/crystal/src/crystal/main.cr:115:7 in 'main'
  from /home/linuxbrew/.linuxbrew/Cellar/crystal/1.12.1_1/share/crystal/src/crystal/main.cr:141:3 in 'main'
  from /lib/x86_64-linux-gnu/libc.so.6 in '??'
  from /lib/x86_64-linux-gnu/libc.so.6 in '__libc_start_main'
  from bin/drift in '_start'
  from ???

Unfortunately, I don't think there's a simple solution to fix this. You would either need to parse the SQL syntax to understand where there's truely a statement end. Or, even better, we should be able to run all statements at once. This is currently only implemented as a custom extension in the postgres driver AFAIK. There's a tracking issue for crystal-db: crystal-lang/crystal-db#113

Maybe a good start would be to mention this trap in the documentation.

[luislavena]

Hello @straight-shoota, thanks for reporting this.

Since I'm not relying on user-defined functions/procedures (like PostgreSQL), I haven't invested any thought on this beyond the "this could happen" scenario 😅

A quick workaround I thought about this was to use a marker to indicate that the migration contains commas and needs to avoid splitting them at that marker.

I will keep this open for the time being, but not sure how I will tackle it, so open to suggestions.

Cheers.