From eb12e019ac29f2cb33336b6782455131d36c68ba Mon Sep 17 00:00:00 2001 From: Matt Spilchen Date: Wed, 7 Jan 2026 14:14:51 -0400 Subject: [PATCH] workload: treat rename destination conflicts as potential errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../schemachange/operation_generator.go | 63 +++++++++---------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 788b7981e61a..04dfc5bdbcac 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -2251,21 +2251,21 @@ func (og *operationGenerator) renameColumn(ctx context.Context, tx pgx.Tx) (*opS if err != nil { return nil, err } - destColumnExists, err := og.columnExistsOnTable(ctx, tx, tableName, destColumnName) - if err != nil { - return nil, err - } stmt := makeOpStmt(OpStmtDDL) stmt.expectedExecErrors.addAll(codesWithConditions{ {pgcode.UndefinedColumn, !srcColumnExists}, - {pgcode.DuplicateColumn, destColumnExists && srcColumnName != destColumnName}, }) - // The column may be referenced in a view or trigger, which can lead to a - // dependency error. This is particularly hard to detect in cases where renaming - // a column that is part of a hash-sharded primary key triggers a cascading rename - // of the crdb_internal shard column, which might be used indirectly by other objects. - stmt.potentialExecErrors.add(pgcode.DependentObjectsStillExist) + stmt.potentialExecErrors.addAll(codesWithConditions{ + // The column may be referenced in a view or trigger, which can lead to a + // dependency error. This is particularly hard to detect in cases where renaming + // a column that is part of a hash-sharded primary key triggers a cascading rename + // of the crdb_internal shard column, which might be used indirectly by other objects. + {code: pgcode.DependentObjectsStillExist, condition: true}, + // Duplicate name errors are potential because destination name conflicts + // can change after checking. + {code: pgcode.DuplicateColumn, condition: srcColumnName != destColumnName}, + }) stmt.sql = fmt.Sprintf(`ALTER TABLE %s RENAME COLUMN %s TO %s`, tableName.String(), srcColumnName.String(), destColumnName.String()) @@ -2307,15 +2307,15 @@ func (og *operationGenerator) renameIndex(ctx context.Context, tx pgx.Tx) (*opSt if err != nil { return nil, err } - destIndexExists, err := og.indexExists(ctx, tx, tableName, destIndexName) - if err != nil { - return nil, err - } stmt := makeOpStmt(OpStmtDDL) stmt.expectedExecErrors.addAll(codesWithConditions{ {code: pgcode.UndefinedObject, condition: !srcIndexExists}, - {code: pgcode.DuplicateRelation, condition: destIndexExists && srcIndexName != destIndexName}, + }) + // Duplicate name errors are potential because destination name conflicts can + // change after checking. + stmt.potentialExecErrors.addAll(codesWithConditions{ + {code: pgcode.DuplicateRelation, condition: srcIndexName != destIndexName}, }) 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 return nil, err } - destSequenceExists, err := og.sequenceExists(ctx, tx, destSequenceName) - if err != nil { - return nil, err - } - srcEqualsDest := srcSequenceName.String() == destSequenceName.String() stmt := makeOpStmt(OpStmtDDL) stmt.expectedExecErrors.addAll(codesWithConditions{ {code: pgcode.UndefinedTable, condition: !srcSequenceExists}, {code: pgcode.UndefinedSchema, condition: !destSchemaExists}, - {code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destSequenceExists}, {code: pgcode.InvalidName, condition: srcSequenceName.Schema() != destSequenceName.Schema()}, }) + // Duplicate name errors are potential because destination name conflicts can + // change after checking. + stmt.potentialExecErrors.addAll(codesWithConditions{ + {code: pgcode.DuplicateRelation, condition: !srcEqualsDest}, + }) stmt.sql = fmt.Sprintf(`ALTER SEQUENCE %s RENAME TO %s`, srcSequenceName, destSequenceName) return stmt, nil @@ -2400,11 +2399,6 @@ func (og *operationGenerator) renameTable(ctx context.Context, tx pgx.Tx) (*opSt return nil, err } - destTableExists, err := og.tableExists(ctx, tx, destTableName) - if err != nil { - return nil, err - } - srcTableHasDependencies, err := og.tableHasDependencies(ctx, tx, srcTableName, false, /* includeFKs */ false /* skipSelfRef */) if err != nil { @@ -2416,10 +2410,14 @@ func (og *operationGenerator) renameTable(ctx context.Context, tx pgx.Tx) (*opSt stmt.expectedExecErrors.addAll(codesWithConditions{ {code: pgcode.UndefinedTable, condition: !srcTableExists}, {code: pgcode.UndefinedSchema, condition: !destSchemaExists}, - {code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destTableExists}, {code: pgcode.DependentObjectsStillExist, condition: srcTableHasDependencies}, {code: pgcode.InvalidName, condition: srcTableName.Schema() != destTableName.Schema()}, }) + // Duplicate name errors are potential because destination name conflicts can + // change after checking. + stmt.potentialExecErrors.addAll(codesWithConditions{ + {code: pgcode.DuplicateRelation, condition: !srcEqualsDest}, + }) stmt.sql = fmt.Sprintf(`ALTER TABLE %s RENAME TO %s`, srcTableName, destTableName) return stmt, nil @@ -2451,11 +2449,6 @@ func (og *operationGenerator) renameView(ctx context.Context, tx pgx.Tx) (*opStm return nil, err } - destViewExists, err := og.viewExists(ctx, tx, destViewName) - if err != nil { - return nil, err - } - srcTableHasDependencies, err := og.tableHasDependencies(ctx, tx, srcViewName, true, /* includeFKs */ false /* skipSelfRef */) if err != nil { @@ -2467,10 +2460,14 @@ func (og *operationGenerator) renameView(ctx context.Context, tx pgx.Tx) (*opStm stmt.expectedExecErrors.addAll(codesWithConditions{ {code: pgcode.UndefinedTable, condition: !srcViewExists}, {code: pgcode.UndefinedSchema, condition: !destSchemaExists}, - {code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destViewExists}, {code: pgcode.DependentObjectsStillExist, condition: srcTableHasDependencies}, {code: pgcode.InvalidName, condition: srcViewName.Schema() != destViewName.Schema()}, }) + // Duplicate name errors are potential because destination name conflicts can + // change after checking. + stmt.potentialExecErrors.addAll(codesWithConditions{ + {code: pgcode.DuplicateRelation, condition: !srcEqualsDest}, + }) stmt.sql = fmt.Sprintf(`ALTER VIEW %s RENAME TO %s`, srcViewName, destViewName) return stmt, nil