Harden --migration_table_name: validation, naming consistency, and truncate support - #163
Merged
sinmetal merged 4 commits intoAug 7, 2026
Conversation
The --migration-table-name flag turned a compile-time constant into user
input that is interpolated into SQL and DDL statements.
- Validate the name against ^[A-Za-z][A-Za-z0-9_]{0,127}$ in
getMigrationTableName, before connecting to Cloud Spanner
- Fall back to the default SchemaMigrations when the flag is empty,
consistently with schemaFilePath
- Quote the table name with backticks in GetSchemaMigrationVersion and
EnsureMigrationTable, as TruncateAllTables already does
- Resolve the table name once per command instead of on every use
All other multi-word flags in wrench use snake_case (credentials_file, schema_file, proto_descriptor_file). The flag is not included in any release yet, so it is renamed without an alias.
TruncateAllTables excluded the literal SchemaMigrations table, so a database using --migration_table_name lost its migration version on truncate and the next migrate up re-applied every migration. - Pass the migration table name to TruncateAllTables - Add --migration_table_name to the truncate command - Move getMigrationTableName to cmd.go, it is no longer migrate specific - Add TestTruncateAllTables covering the default and a custom table name
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
sinmetal
force-pushed
the
fix-migration-table-name-validation
branch
from
August 5, 2026 05:55
6337f22 to
c57af8d
Compare
sinmetal
marked this pull request as ready for review
August 5, 2026 05:59
Cloud Spanner identifiers are case insensitive, but INFORMATION_SCHEMA returns the declared name. --migration_table_name schemamigrations therefore resolved to the existing SchemaMigrations table everywhere except the exclusion check in TruncateAllTables, which deleted the migration version.
vvakame
approved these changes
Aug 7, 2026
vvakame
left a comment
Collaborator
There was a problem hiding this comment.
LGTM.
Note: This changes contains breaking change.
Collaborator
|
@sinmetal Sorry for the late look — catching up after the merge. Thanks for the fix! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY
#152 added the
--migration-table-nameflag. It works, but it left three issues behind.The table name is interpolated into SQL/DDL without any validation.
GetSchemaMigrationVersionbuilds"SELECT Version, Dirty FROM " + tableName + " LIMIT 1"and
EnsureMigrationTablebuildsfmt.Sprintf("CREATE TABLE %s ..."). Until Add --migration-table-name flag to migrate command #152 thename was a compile-time constant, so this was safe. Now it is user input, and a value
passed from a CI variable can change the statement. Passing an empty string also produced
a confusing
CREATE TABLE (error from Cloud Spanner instead of a clear message.The flag name does not follow the existing convention.
Every other multi-word flag in wrench uses snake_case:
credentials_file,schema_file,proto_descriptor_file. Only this one was kebab-case.truncatesilently destroys the migration version when a custom table is used.TruncateAllTablesskips the literalSchemaMigrationstable so that a truncateddatabase keeps its migration version. With a custom table name, that exclusion does not
match, the version row is deleted, and the next
migrate upre-applies every migrationfrom version 0 — failing with errors like
Duplicate column name ...for DDL migrations.This is easy to hit because
truncateis a routine command in development and CI, andthe failure surfaces later in an unrelated command.
WHAT
Validate the table name and quote it (507e775)
^[A-Za-z][A-Za-z0-9_]{0,127}$ingetMigrationTableName, beforeconnecting to Cloud Spanner, matching the Cloud Spanner table name rules
SchemaMigrationswhen the flag is empty, consistently withschemaFilePath()GetSchemaMigrationVersionandEnsureMigrationTable, asTruncateAllTablesalready doesRename the flag to
--migration_table_name(6ec6969)v1.13.5), so it is renamedwithout keeping an alias
Keep the specified migration table on
truncate(6337f22)TruncateAllTablesnow takes the migration table name and excludes that tabletruncateaccepts--migration_table_namegetMigrationTableNamemoved tocmd/cmd.gosince it is no longer migrate specificDocs are updated: the README describes the name restriction, and notes that the same
--migration_table_namevalue must be given tomigrate up/version/setandtruncate.Testing
TestGetMigrationTableNamecovers the empty fallback, a leading digit, an invalidcharacter, a backtick-based injection string, and a name longer than 128 characters
TestTruncateAllTablesis new and covers both the default and a custom table name.Verified that it fails against the previous hardcoded exclusion
truncate --migration_table_name Data_Migrationskeeps the version, while
truncatewithout the flag drops it toNo migrations.ion why you submit this pull request-->