Skip to content

Commit eb12e01

Browse files
committed
workload: treat rename destination conflicts as potential errors
Rename operations (e.g., sequences, tables) check for name conflicts at snapshot time but execute later, allowing concurrent transactions to change the destination state. This can cause expected errors (e.g., DuplicateRelation) to not occur, leading to false workload failures. The workload now treats such cases as potential errors instead of hard failures. Example: ``` TX1: Checks seq_w18_32 exists → TRUE TX1: Expects error on renaming to seq_w18_32 TX2: Renames seq_w18_32 away → commits TX1: Rename unexpectedly succeeds - workload fails: error was expected but not raised ``` Closes #160465 Release note: none Epic: none
1 parent 75d66c3 commit eb12e01

File tree

1 file changed

+30
-33
lines changed

1 file changed

+30
-33
lines changed

pkg/workload/schemachange/operation_generator.go

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2251,21 +2251,21 @@ func (og *operationGenerator) renameColumn(ctx context.Context, tx pgx.Tx) (*opS
22512251
if err != nil {
22522252
return nil, err
22532253
}
2254-
destColumnExists, err := og.columnExistsOnTable(ctx, tx, tableName, destColumnName)
2255-
if err != nil {
2256-
return nil, err
2257-
}
22582254

22592255
stmt := makeOpStmt(OpStmtDDL)
22602256
stmt.expectedExecErrors.addAll(codesWithConditions{
22612257
{pgcode.UndefinedColumn, !srcColumnExists},
2262-
{pgcode.DuplicateColumn, destColumnExists && srcColumnName != destColumnName},
22632258
})
2264-
// The column may be referenced in a view or trigger, which can lead to a
2265-
// dependency error. This is particularly hard to detect in cases where renaming
2266-
// a column that is part of a hash-sharded primary key triggers a cascading rename
2267-
// of the crdb_internal shard column, which might be used indirectly by other objects.
2268-
stmt.potentialExecErrors.add(pgcode.DependentObjectsStillExist)
2259+
stmt.potentialExecErrors.addAll(codesWithConditions{
2260+
// The column may be referenced in a view or trigger, which can lead to a
2261+
// dependency error. This is particularly hard to detect in cases where renaming
2262+
// a column that is part of a hash-sharded primary key triggers a cascading rename
2263+
// of the crdb_internal shard column, which might be used indirectly by other objects.
2264+
{code: pgcode.DependentObjectsStillExist, condition: true},
2265+
// Duplicate name errors are potential because destination name conflicts
2266+
// can change after checking.
2267+
{code: pgcode.DuplicateColumn, condition: srcColumnName != destColumnName},
2268+
})
22692269

22702270
stmt.sql = fmt.Sprintf(`ALTER TABLE %s RENAME COLUMN %s TO %s`,
22712271
tableName.String(), srcColumnName.String(), destColumnName.String())
@@ -2307,15 +2307,15 @@ func (og *operationGenerator) renameIndex(ctx context.Context, tx pgx.Tx) (*opSt
23072307
if err != nil {
23082308
return nil, err
23092309
}
2310-
destIndexExists, err := og.indexExists(ctx, tx, tableName, destIndexName)
2311-
if err != nil {
2312-
return nil, err
2313-
}
23142310

23152311
stmt := makeOpStmt(OpStmtDDL)
23162312
stmt.expectedExecErrors.addAll(codesWithConditions{
23172313
{code: pgcode.UndefinedObject, condition: !srcIndexExists},
2318-
{code: pgcode.DuplicateRelation, condition: destIndexExists && srcIndexName != destIndexName},
2314+
})
2315+
// Duplicate name errors are potential because destination name conflicts can
2316+
// change after checking.
2317+
stmt.potentialExecErrors.addAll(codesWithConditions{
2318+
{code: pgcode.DuplicateRelation, condition: srcIndexName != destIndexName},
23192319
})
23202320

23212321
stmt.sql = fmt.Sprintf(`ALTER INDEX %s@"%s" RENAME TO "%s"`,
@@ -2350,19 +2350,18 @@ func (og *operationGenerator) renameSequence(ctx context.Context, tx pgx.Tx) (*o
23502350
return nil, err
23512351
}
23522352

2353-
destSequenceExists, err := og.sequenceExists(ctx, tx, destSequenceName)
2354-
if err != nil {
2355-
return nil, err
2356-
}
2357-
23582353
srcEqualsDest := srcSequenceName.String() == destSequenceName.String()
23592354
stmt := makeOpStmt(OpStmtDDL)
23602355
stmt.expectedExecErrors.addAll(codesWithConditions{
23612356
{code: pgcode.UndefinedTable, condition: !srcSequenceExists},
23622357
{code: pgcode.UndefinedSchema, condition: !destSchemaExists},
2363-
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destSequenceExists},
23642358
{code: pgcode.InvalidName, condition: srcSequenceName.Schema() != destSequenceName.Schema()},
23652359
})
2360+
// Duplicate name errors are potential because destination name conflicts can
2361+
// change after checking.
2362+
stmt.potentialExecErrors.addAll(codesWithConditions{
2363+
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest},
2364+
})
23662365

23672366
stmt.sql = fmt.Sprintf(`ALTER SEQUENCE %s RENAME TO %s`, srcSequenceName, destSequenceName)
23682367
return stmt, nil
@@ -2400,11 +2399,6 @@ func (og *operationGenerator) renameTable(ctx context.Context, tx pgx.Tx) (*opSt
24002399
return nil, err
24012400
}
24022401

2403-
destTableExists, err := og.tableExists(ctx, tx, destTableName)
2404-
if err != nil {
2405-
return nil, err
2406-
}
2407-
24082402
srcTableHasDependencies, err := og.tableHasDependencies(ctx, tx, srcTableName, false, /* includeFKs */
24092403
false /* skipSelfRef */)
24102404
if err != nil {
@@ -2416,10 +2410,14 @@ func (og *operationGenerator) renameTable(ctx context.Context, tx pgx.Tx) (*opSt
24162410
stmt.expectedExecErrors.addAll(codesWithConditions{
24172411
{code: pgcode.UndefinedTable, condition: !srcTableExists},
24182412
{code: pgcode.UndefinedSchema, condition: !destSchemaExists},
2419-
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destTableExists},
24202413
{code: pgcode.DependentObjectsStillExist, condition: srcTableHasDependencies},
24212414
{code: pgcode.InvalidName, condition: srcTableName.Schema() != destTableName.Schema()},
24222415
})
2416+
// Duplicate name errors are potential because destination name conflicts can
2417+
// change after checking.
2418+
stmt.potentialExecErrors.addAll(codesWithConditions{
2419+
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest},
2420+
})
24232421

24242422
stmt.sql = fmt.Sprintf(`ALTER TABLE %s RENAME TO %s`, srcTableName, destTableName)
24252423
return stmt, nil
@@ -2451,11 +2449,6 @@ func (og *operationGenerator) renameView(ctx context.Context, tx pgx.Tx) (*opStm
24512449
return nil, err
24522450
}
24532451

2454-
destViewExists, err := og.viewExists(ctx, tx, destViewName)
2455-
if err != nil {
2456-
return nil, err
2457-
}
2458-
24592452
srcTableHasDependencies, err := og.tableHasDependencies(ctx, tx, srcViewName, true, /* includeFKs */
24602453
false /* skipSelfRef */)
24612454
if err != nil {
@@ -2467,10 +2460,14 @@ func (og *operationGenerator) renameView(ctx context.Context, tx pgx.Tx) (*opStm
24672460
stmt.expectedExecErrors.addAll(codesWithConditions{
24682461
{code: pgcode.UndefinedTable, condition: !srcViewExists},
24692462
{code: pgcode.UndefinedSchema, condition: !destSchemaExists},
2470-
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destViewExists},
24712463
{code: pgcode.DependentObjectsStillExist, condition: srcTableHasDependencies},
24722464
{code: pgcode.InvalidName, condition: srcViewName.Schema() != destViewName.Schema()},
24732465
})
2466+
// Duplicate name errors are potential because destination name conflicts can
2467+
// change after checking.
2468+
stmt.potentialExecErrors.addAll(codesWithConditions{
2469+
{code: pgcode.DuplicateRelation, condition: !srcEqualsDest},
2470+
})
24742471

24752472
stmt.sql = fmt.Sprintf(`ALTER VIEW %s RENAME TO %s`, srcViewName, destViewName)
24762473
return stmt, nil

0 commit comments

Comments
 (0)