diff --git a/go/libraries/doltcore/dconfig/envvars.go b/go/libraries/doltcore/dconfig/envvars.go index 9c7d11f8e4..47552ba274 100755 --- a/go/libraries/doltcore/dconfig/envvars.go +++ b/go/libraries/doltcore/dconfig/envvars.go @@ -49,4 +49,7 @@ const ( // If set, must be "kill_connections" or "session_aware" // Will go away after session_aware is made default-and-only. EnvGCSafepointControllerChoice = "DOLT_GC_SAFEPOINT_CONTROLLER_CHOICE" + + // Used for tests. If set, Dolt will error if it would rewrite a table hash. + EnvAssertNoTableRewrite = "DOLT_REST_ASSERT_NO_TABLE_REWRITE" ) diff --git a/go/libraries/doltcore/sqle/tables.go b/go/libraries/doltcore/sqle/tables.go index c6ef4bba28..537f62fd5b 100644 --- a/go/libraries/doltcore/sqle/tables.go +++ b/go/libraries/doltcore/sqle/tables.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "github.com/dolthub/dolt/go/libraries/doltcore/dconfig" "io" "math" "os" @@ -1725,6 +1726,9 @@ func (t *AlterableDoltTable) RewriteInserter( newColumn *sql.Column, idxCols []sql.IndexColumn, ) (sql.RowInserter, error) { + if _, isSet := os.LookupEnv(dconfig.EnvAssertNoTableRewrite); isSet { + return nil, fmt.Errorf("attempted to rewrite table but %s was set", dconfig.EnvAssertNoTableRewrite) + } if err := dsess.CheckAccessForDb(ctx, t.db, branch_control.Permissions_Write); err != nil { return nil, err } diff --git a/integration-tests/bats/schema-changes.bats b/integration-tests/bats/schema-changes.bats index dcb41d71f7..1e35f6d793 100755 --- a/integration-tests/bats/schema-changes.bats +++ b/integration-tests/bats/schema-changes.bats @@ -395,3 +395,60 @@ EOF [ $status -eq 0 ] [[ $output =~ "CHECK ((\`col1\` = 'valid'))" ]] || false } + +@test "schema-changes: assert that schema changes don't rewrite the table when they don't need to" { + dolt sql -q "create table t (pk int primary key, e enum('a', 'b', 'c'), s set('a', 'b', 'c'), c char(10), vc varchar(10), b binary(10), vb varbinary(10));" + dolt sql -q "insert into t values (1, 'a', 'a,b', 'c', 'vc', 'b', 'vb');" + dolt commit -Am "initial" + + # First check that DOLT_REST_ASSERT_NO_TABLE_REWRITE does prevent schema changes that *do* require a table rewrite. + # Although some of these operations don't change any rows, determining this requires an attempted rewrite that scans the table. + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column e enum('a', 'b');" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column s set('a', 'b');" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column c char(5);" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vc varchar(5);" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + # Unlike the other string types, binary requires a rewrite even when making the length longer. + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column b binary(15);" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vb varbinary(5);" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + # Changing the character set requires a rewrite. + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vc varbinary(10);" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + # Changing the character set requires a rewrite. + run env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vc varchar(10) collate latin1_german1_ci;" + [ $status -ne 0 ] + [[ $output =~ "attempted to rewrite table but DOLT_REST_ASSERT_NO_TABLE_REWRITE was set" ]] || false + + env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column e enum('a', 'b', 'c', 'd', 'e');" + env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column s set('a', 'b', 'c', 'd', 'e');" + env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column c char(15);" + env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vc varchar(15);" + env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vb varbinary(15);" + + # Changing the collation doesn't require a table rewrite, but it does invalidate the secondary indexes. + # The GMS API doesn't currently less us separate the two, so currently this causes a table rewrite, but it doesn't have to. + # env DOLT_REST_ASSERT_NO_TABLE_REWRITE=1 dolt sql -q "alter table t modify column vc varchar(15) collate utf8mb4_0900_ai_ci;" + + # After all these schema changes, the table hash remains the same. + run dolt sql -r csv -q "select DOLT_HASHOF_TABLE('t') = (select DOLT_HASHOF_TABLE('t') from t as of HEAD);" + [[ "$output" =~ "true" ]] || false +} \ No newline at end of file