Skip to content

Fix: #340 Add comment handling and related tests for migration functionality#344

Merged
dkropachev merged 1 commit into
scylladb:masterfrom
itsfuad:master
Sep 13, 2025
Merged

Fix: #340 Add comment handling and related tests for migration functionality#344
dkropachev merged 1 commit into
scylladb:masterfrom
itsfuad:master

Conversation

@itsfuad

@itsfuad itsfuad commented Sep 13, 2025

Copy link
Copy Markdown
Contributor

This pull request improves the migration system by introducing logic to correctly handle SQL comments in migration files, ensuring that regular comments (including trailing comments after statements) are ignored during migration execution. It also adds comprehensive tests to verify this behavior and documents the new logic for clarity.

Migration logic improvements:

  • Added the isComment function to distinguish regular SQL comments from callback commands, ensuring only valid statements and callbacks are processed during migrations (migrate/migrate.go).
  • Updated the migration execution flow so that regular comments and empty statements are silently skipped, preventing errors from trailing comments in migration files (migrate/migrate.go).

Testing enhancements:

  • Added the TestMigrationWithTrailingComment test to verify that migrations with trailing comments succeed without error (migrate/migrate_test.go).
  • Added the TestIsComment test suite to validate the behavior of the new isComment function for various statement types (migrate/migrate_test.go).
  • Exposed the IsComment function for use in tests and other packages (migrate/export_test.go).

Documentation updates:

  • Documented the new migration logic and statement handling in the comments for the applyMigration function, clarifying the distinction between SQL statements, callback commands, and regular comments (migrate/migrate.go).

What fixed
#340

@dkropachev

Copy link
Copy Markdown
Contributor

@itsfuad , thanks, what a great PR.

@dkropachev dkropachev merged commit 101d498 into scylladb:master Sep 13, 2025
2 checks passed
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